From 76a2640ffee9a69c927a4ddd8f49be7b255f17ab Mon Sep 17 00:00:00 2001 From: Francisco Rolotti <102830313+franrolotti@users.noreply.github.com> Date: Mon, 16 Feb 2026 11:48:17 +0100 Subject: [PATCH 1/9] fix(deploy): prevent nested ProxyAdmin ownership and enforce direct admin control --- script/Common.sol | 74 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 65 insertions(+), 9 deletions(-) diff --git a/script/Common.sol b/script/Common.sol index 90a9112..78ffd0a 100644 --- a/script/Common.sol +++ b/script/Common.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.28; import {ProxyAdmin} from '@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol'; import {TransparentUpgradeableProxy} from '@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol'; import {Script} from 'forge-std/Script.sol'; +import {console2} from 'forge-std/console2.sol'; import {DelegatedSavingCircles} from '../src/contracts/DelegatedSavingCircles.sol'; import {SavingCircles} from '../src/contracts/SavingCircles.sol'; @@ -16,35 +17,90 @@ import {SavingCirclesViewer} from '../src/contracts/SavingCirclesViewer.sol'; * @dev This contract is intended for use in Scripts and Integration Tests */ contract Common is Script { + bytes32 internal constant _ERC1967_IMPLEMENTATION_SLOT = + 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; + bytes32 internal constant _ERC1967_ADMIN_SLOT = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103; + bytes4 internal constant _OWNER_SELECTOR = bytes4(keccak256('owner()')); + bytes4 internal constant _UPGRADE_INTERFACE_VERSION_SELECTOR = bytes4(keccak256('UPGRADE_INTERFACE_VERSION()')); + + error InvalidAdminAddress(); + error InvalidAdminProxyAdmin(address admin); + error ProxyAdminOwnerMismatch(address proxyAdmin, address expectedOwner, address actualOwner); + error ProxyAdminNotDeployed(address proxy); + error ProxyImplementationSlotMismatch(address proxy, address expectedImplementation, address actualImplementation); + function setUp() public virtual {} function _deploySavingCircles() internal returns (SavingCircles) { return new SavingCircles(); } - function _deployProxyAdmin(address _admin) internal returns (ProxyAdmin) { - return new ProxyAdmin(_admin); - } - function _deployTransparentProxy( address _implementation, - address _proxyAdmin, + address _adminOwner, bytes memory _initData ) internal returns (TransparentUpgradeableProxy) { - return new TransparentUpgradeableProxy(_implementation, _proxyAdmin, _initData); + return new TransparentUpgradeableProxy(_implementation, _adminOwner, _initData); } function _deployContracts(address _admin) internal returns (TransparentUpgradeableProxy) { + _assertValidAdmin(_admin); + + SavingCircles implementation = _deploySavingCircles(); TransparentUpgradeableProxy proxy = _deployTransparentProxy( - address(_deploySavingCircles()), - address(_deployProxyAdmin(_admin)), - abi.encodeWithSelector(SavingCircles.initialize.selector, _admin) + address(implementation), _admin, abi.encodeWithSelector(SavingCircles.initialize.selector, _admin) ); // Deploy auxiliary contracts that reference the SavingCircles proxy new DelegatedSavingCircles(address(proxy)); new SavingCirclesViewer(address(proxy)); + address proxyAdmin = _assertDeployment(address(proxy), address(implementation), _admin); + + console2.log('Deployer', msg.sender); + console2.log('Admin', _admin); + console2.log('ProxyAdmin', proxyAdmin); + console2.log('Proxy', address(proxy)); + console2.log('Implementation', address(implementation)); + return proxy; } + + function _assertValidAdmin(address _admin) internal view { + if (_admin == address(0)) revert InvalidAdminAddress(); + if (_isProxyAdmin(_admin)) revert InvalidAdminProxyAdmin(_admin); + } + + function _assertDeployment( + address _proxy, + address _implementation, + address _expectedAdminOwner + ) internal view returns (address proxyAdmin) { + proxyAdmin = _readAddressFromSlot(_proxy, _ERC1967_ADMIN_SLOT); + if (proxyAdmin == address(0)) revert ProxyAdminNotDeployed(_proxy); + + address actualOwner = ProxyAdmin(proxyAdmin).owner(); + if (actualOwner != _expectedAdminOwner) { + revert ProxyAdminOwnerMismatch(proxyAdmin, _expectedAdminOwner, actualOwner); + } + + address actualImplementation = _readAddressFromSlot(_proxy, _ERC1967_IMPLEMENTATION_SLOT); + if (actualImplementation != _implementation) { + revert ProxyImplementationSlotMismatch(_proxy, _implementation, actualImplementation); + } + } + + function _readAddressFromSlot(address _contract, bytes32 _slot) internal view returns (address) { + return address(uint160(uint256(vm.load(_contract, _slot)))); + } + + function _isProxyAdmin(address _candidate) internal view returns (bool) { + if (_candidate.code.length == 0) return false; + + (bool ownerCallSuccess,) = _candidate.staticcall(abi.encodeWithSelector(_OWNER_SELECTOR)); + if (!ownerCallSuccess) return false; + + (bool versionCallSuccess,) = _candidate.staticcall(abi.encodeWithSelector(_UPGRADE_INTERFACE_VERSION_SELECTOR)); + return versionCallSuccess; + } } From 740e5321be3f988934c528877cb7866ff998d883 Mon Sep 17 00:00:00 2001 From: Francisco Rolotti <102830313+franrolotti@users.noreply.github.com> Date: Mon, 9 Mar 2026 10:09:43 +0100 Subject: [PATCH 2/9] feat(script): add read-only proxy upgrade validation script with admin/implementation guardrails --- script/UpgradeValidate.s.sol | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 script/UpgradeValidate.s.sol diff --git a/script/UpgradeValidate.s.sol b/script/UpgradeValidate.s.sol new file mode 100644 index 0000000..9649653 --- /dev/null +++ b/script/UpgradeValidate.s.sol @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.28; + +import {ProxyAdmin} from '@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol'; +import {console2} from 'forge-std/console2.sol'; + +import {Common} from 'script/Common.sol'; +import {SavingCircles} from 'src/contracts/SavingCircles.sol'; + +contract UpgradeValidate is Common { + error NewImplementationHasNoCode(address implementation); + error NewImplementationMatchesCurrent(address implementation); + + function run() public view { + address proxy = vm.envAddress('PROXY_ADDRESS'); + address expectedAdminOwner = vm.envAddress('EXPECTED_ADMIN_OWNER'); + address expectedCurrentImplementation = vm.envAddress('EXPECTED_CURRENT_IMPLEMENTATION'); + address newImplementation = vm.envAddress('NEW_IMPLEMENTATION'); + + address proxyAdmin = _assertDeployment(proxy, expectedCurrentImplementation, expectedAdminOwner); + + if (newImplementation.code.length == 0) revert NewImplementationHasNoCode(newImplementation); + if (newImplementation == expectedCurrentImplementation) { + revert NewImplementationMatchesCurrent(newImplementation); + } + + // Lightweight smoke check against proxy to confirm implementation responds. + uint256 nextId = SavingCircles(proxy).nextId(); + + console2.log('Validation successful'); + console2.log('Proxy', proxy); + console2.log('ProxyAdmin', proxyAdmin); + console2.log('AdminOwner', ProxyAdmin(proxyAdmin).owner()); + console2.log('CurrentImplementation', expectedCurrentImplementation); + console2.log('NewImplementation', newImplementation); + console2.log('SmokeCheck.nextId', nextId); + } +} From f13baa526fd2f120600bb8649a7f0ddada55159d Mon Sep 17 00:00:00 2001 From: Francisco Rolotti <102830313+franrolotti@users.noreply.github.com> Date: Mon, 9 Mar 2026 11:22:29 +0100 Subject: [PATCH 3/9] feat(script): add upgrade execute and post-upgrade validation scripts --- script/UpgradeExecute.s.sol | 61 ++++++++++++++++++++++++++++++++ script/UpgradePostValidate.s.sol | 50 ++++++++++++++++++++++++++ script/UpgradeValidate.s.sol | 3 +- 3 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 script/UpgradeExecute.s.sol create mode 100644 script/UpgradePostValidate.s.sol diff --git a/script/UpgradeExecute.s.sol b/script/UpgradeExecute.s.sol new file mode 100644 index 0000000..abe638e --- /dev/null +++ b/script/UpgradeExecute.s.sol @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.28; + +import {ProxyAdmin} from '@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol'; +import {ITransparentUpgradeableProxy} from '@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol'; +import {console2} from 'forge-std/console2.sol'; + +import {Common} from 'script/Common.sol'; + +contract UpgradeExecute is Common { + error NewImplementationHasNoCode(address implementation); + error NewImplementationMatchesCurrent(address implementation); + error AlreadyUpgraded(address implementation); + + function run() public { + address proxy = vm.envAddress('PROXY_ADDRESS'); + address expectedAdminOwner = vm.envAddress('EXPECTED_ADMIN_OWNER'); + address expectedCurrentImplementation = vm.envAddress('EXPECTED_CURRENT_IMPLEMENTATION'); + address newImplementation = vm.envAddress('NEW_IMPLEMENTATION'); + bytes memory upgradeCalldata = _upgradeCalldataOrEmpty(); + + address currentImplementation = _readAddressFromSlot(proxy, _ERC1967_IMPLEMENTATION_SLOT); + if (currentImplementation == newImplementation) revert AlreadyUpgraded(newImplementation); + + address proxyAdmin = _assertDeployment(proxy, expectedCurrentImplementation, expectedAdminOwner); + + if (newImplementation.code.length == 0) revert NewImplementationHasNoCode(newImplementation); + if (newImplementation == expectedCurrentImplementation) { + revert NewImplementationMatchesCurrent(newImplementation); + } + + console2.log('Executing upgrade'); + console2.log('Proxy', proxy); + console2.log('ProxyAdmin', proxyAdmin); + console2.log('AdminOwner', expectedAdminOwner); + console2.log('CurrentImplementation', currentImplementation); + console2.log('NewImplementation', newImplementation); + console2.log('CalldataLength'); + console2.log(upgradeCalldata.length); + + vm.startBroadcast(); + ProxyAdmin(proxyAdmin) + .upgradeAndCall(ITransparentUpgradeableProxy(payable(proxy)), newImplementation, upgradeCalldata); + vm.stopBroadcast(); + + _assertDeployment(proxy, newImplementation, expectedAdminOwner); + + console2.log('Upgrade successful'); + console2.log('Proxy', proxy); + console2.log('ProxyAdmin', proxyAdmin); + console2.log('CurrentImplementation', _readAddressFromSlot(proxy, _ERC1967_IMPLEMENTATION_SLOT)); + } + + function _upgradeCalldataOrEmpty() internal view returns (bytes memory data) { + try vm.envBytes('UPGRADE_CALLDATA') returns (bytes memory envData) { + return envData; + } catch { + return bytes(''); + } + } +} diff --git a/script/UpgradePostValidate.s.sol b/script/UpgradePostValidate.s.sol new file mode 100644 index 0000000..6526399 --- /dev/null +++ b/script/UpgradePostValidate.s.sol @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.28; + +import {ProxyAdmin} from '@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol'; +import {console2} from 'forge-std/console2.sol'; + +import {Common} from 'script/Common.sol'; +import {SavingCircles} from 'src/contracts/SavingCircles.sol'; +import {SavingCirclesViewer} from 'src/contracts/SavingCirclesViewer.sol'; + +contract UpgradePostValidate is Common { + error ViewerProxyMismatch(address viewer, address expectedProxy, address actualProxy); + + function run() public view { + address proxy = vm.envAddress('PROXY_ADDRESS'); + address expectedAdminOwner = vm.envAddress('EXPECTED_ADMIN_OWNER'); + address expectedImplementation = vm.envAddress('EXPECTED_IMPLEMENTATION'); + + address proxyAdmin = _assertDeployment(proxy, expectedImplementation, expectedAdminOwner); + uint256 nextId = SavingCircles(proxy).nextId(); + + console2.log('Post-upgrade validation successful'); + console2.log('Proxy', proxy); + console2.log('ProxyAdmin', proxyAdmin); + console2.log('AdminOwner', ProxyAdmin(proxyAdmin).owner()); + console2.log('Implementation', expectedImplementation); + console2.log('SmokeCheck.nextId'); + console2.log(nextId); + + address viewer = _viewerAddressOrZero(); + if (viewer == address(0)) return; + + address viewerProxy = address(SavingCirclesViewer(viewer).SAVING_CIRCLES()); + if (viewerProxy != proxy) revert ViewerProxyMismatch(viewer, proxy, viewerProxy); + + uint256 totalBalance = SavingCirclesViewer(viewer).getTotalBalance(expectedAdminOwner); + console2.log('Viewer', viewer); + console2.log('Viewer.SAVING_CIRCLES', viewerProxy); + console2.log('ViewerSmoke.totalBalance(owner)'); + console2.log(totalBalance); + } + + function _viewerAddressOrZero() internal view returns (address viewer) { + try vm.envAddress('VIEWER_ADDRESS') returns (address envViewer) { + return envViewer; + } catch { + return address(0); + } + } +} diff --git a/script/UpgradeValidate.s.sol b/script/UpgradeValidate.s.sol index 9649653..e968b41 100644 --- a/script/UpgradeValidate.s.sol +++ b/script/UpgradeValidate.s.sol @@ -33,6 +33,7 @@ contract UpgradeValidate is Common { console2.log('AdminOwner', ProxyAdmin(proxyAdmin).owner()); console2.log('CurrentImplementation', expectedCurrentImplementation); console2.log('NewImplementation', newImplementation); - console2.log('SmokeCheck.nextId', nextId); + console2.log('SmokeCheck.nextId'); + console2.log(nextId); } } From a7ceaffc3e90ac11fca26f1e6f616e4f4296b3e9 Mon Sep 17 00:00:00 2001 From: Francisco Rolotti <102830313+franrolotti@users.noreply.github.com> Date: Mon, 9 Mar 2026 11:23:05 +0100 Subject: [PATCH 4/9] chore(script): add canonical storage layout compatibility checker --- scripts/check_storage_layout.sh | 97 +++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100755 scripts/check_storage_layout.sh diff --git a/scripts/check_storage_layout.sh b/scripts/check_storage_layout.sh new file mode 100755 index 0000000..184cfb8 --- /dev/null +++ b/scripts/check_storage_layout.sh @@ -0,0 +1,97 @@ +#!/usr/bin/env bash +set -euo pipefail + +OLD_REF="${1:-dev}" +NEW_REF="${2:-HEAD}" +CONTRACT_TARGET="${3:-src/contracts/SavingCircles.sol:SavingCircles}" + +ROOT_DIR="$(git rev-parse --show-toplevel)" +TMP_DIR="$(mktemp -d)" +OLD_DIR="$TMP_DIR/old" +NEW_DIR="$TMP_DIR/new" + +cleanup() { + git -C "$ROOT_DIR" worktree remove "$OLD_DIR" --force >/dev/null 2>&1 || true + git -C "$ROOT_DIR" worktree remove "$NEW_DIR" --force >/dev/null 2>&1 || true + rm -rf "$TMP_DIR" +} +trap cleanup EXIT + +link_deps() { + local target_dir="$1" + if [[ -d "$ROOT_DIR/node_modules" && ! -e "$target_dir/node_modules" ]]; then + ln -s "$ROOT_DIR/node_modules" "$target_dir/node_modules" + fi + if [[ -d "$ROOT_DIR/lib" && ! -e "$target_dir/lib" ]]; then + ln -s "$ROOT_DIR/lib" "$target_dir/lib" + fi +} + +inspect_layout() { + local workdir="$1" + local output_file="$2" + local raw_file="$TMP_DIR/raw-$(basename "$output_file").txt" + local canonical_file="$TMP_DIR/canonical-$(basename "$output_file")" + + if ! (cd "$workdir" && forge inspect "$CONTRACT_TARGET" storage-layout --json > "$raw_file"); then + echo "forge inspect failed in $workdir" + cat "$raw_file" + return 1 + fi + + if ! jq -S . < "$raw_file" > "$output_file"; then + echo "Failed to parse forge inspect output as JSON in $workdir" + cat "$raw_file" + return 1 + fi + + # Canonicalize layout by dropping compiler-internal IDs and normalizing type IDs. + if ! jq -S ' + def norm_type: + gsub("t_struct\\(([^)]*)\\)[0-9]+_storage"; "t_struct(\\1)_storage"); + + { + storage: [.storage[] | {label, slot, offset, type: (.type | norm_type)}], + structs: ( + .types + | to_entries + | map(.value) + | map(select(.label | startswith("struct "))) + | map({ + label, + members: [.members[] | {label, slot, offset, type: (.type | norm_type)}] + }) + | sort_by(.label) + ) + } + ' "$output_file" > "$canonical_file"; then + echo "Failed to canonicalize storage layout in $workdir" + cat "$output_file" + return 1 + fi +} + +echo "Comparing storage layout for $CONTRACT_TARGET" +echo "Old ref: $OLD_REF" +echo "New ref: $NEW_REF" + +git -C "$ROOT_DIR" worktree add --detach "$OLD_DIR" "$OLD_REF" >/dev/null +link_deps "$OLD_DIR" +inspect_layout "$OLD_DIR" "$TMP_DIR/old-layout.json" + +if [[ "$NEW_REF" == "HEAD" ]]; then + inspect_layout "$ROOT_DIR" "$TMP_DIR/new-layout.json" +else + git -C "$ROOT_DIR" worktree add --detach "$NEW_DIR" "$NEW_REF" >/dev/null + link_deps "$NEW_DIR" + inspect_layout "$NEW_DIR" "$TMP_DIR/new-layout.json" +fi + +if diff -u \ + "$TMP_DIR/canonical-old-layout.json" \ + "$TMP_DIR/canonical-new-layout.json"; then + echo "Storage layout check passed: no differences detected." +else + echo "Storage layout check failed: differences detected." + exit 1 +fi From 68c1869f9d9c1325b19a24444f404ce3c2a98a0a Mon Sep 17 00:00:00 2001 From: Francisco Rolotti <102830313+franrolotti@users.noreply.github.com> Date: Mon, 9 Mar 2026 11:24:52 +0100 Subject: [PATCH 5/9] ci: add PR storage layout compatibility check for SavingCircles --- .github/workflows/storage-layout-check.yml | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .github/workflows/storage-layout-check.yml diff --git a/.github/workflows/storage-layout-check.yml b/.github/workflows/storage-layout-check.yml new file mode 100644 index 0000000..296e0af --- /dev/null +++ b/.github/workflows/storage-layout-check.yml @@ -0,0 +1,25 @@ +name: Storage Layout Check + +on: + pull_request: + branches: + - main + +jobs: + storage-layout: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + submodules: recursive + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + + - name: Install Node dependencies + run: npm ci + + - name: Check SavingCircles storage layout + run: ./scripts/check_storage_layout.sh ${{ github.event.pull_request.base.sha }} HEAD src/contracts/SavingCircles.sol:SavingCircles From ff77fd098bb9453cf4997885b9a3455e7702c8d7 Mon Sep 17 00:00:00 2001 From: Francisco Rolotti <102830313+franrolotti@users.noreply.github.com> Date: Mon, 9 Mar 2026 11:33:50 +0100 Subject: [PATCH 6/9] ci: compare SavingCircles storage layout against dev for all PRs --- .github/workflows/storage-layout-check.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/storage-layout-check.yml b/.github/workflows/storage-layout-check.yml index 296e0af..ab3fa4d 100644 --- a/.github/workflows/storage-layout-check.yml +++ b/.github/workflows/storage-layout-check.yml @@ -21,5 +21,8 @@ jobs: - name: Install Node dependencies run: npm ci + - name: Fetch dev branch + run: git fetch origin dev:refs/remotes/origin/dev + - name: Check SavingCircles storage layout - run: ./scripts/check_storage_layout.sh ${{ github.event.pull_request.base.sha }} HEAD src/contracts/SavingCircles.sol:SavingCircles + run: ./scripts/check_storage_layout.sh origin/dev HEAD src/contracts/SavingCircles.sol:SavingCircles From 981c11789c5e7ce1804de1fbf6f1fee463cfb7c0 Mon Sep 17 00:00:00 2001 From: Francisco Rolotti <102830313+franrolotti@users.noreply.github.com> Date: Mon, 9 Mar 2026 11:37:07 +0100 Subject: [PATCH 7/9] ci: run storage layout check on pull requests to dev --- .github/workflows/storage-layout-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/storage-layout-check.yml b/.github/workflows/storage-layout-check.yml index ab3fa4d..5f67fd7 100644 --- a/.github/workflows/storage-layout-check.yml +++ b/.github/workflows/storage-layout-check.yml @@ -3,7 +3,7 @@ name: Storage Layout Check on: pull_request: branches: - - main + - dev jobs: storage-layout: From 5a33376c64b823185c2dfa81209152c02c330ea2 Mon Sep 17 00:00:00 2001 From: Francisco Rolotti <102830313+franrolotti@users.noreply.github.com> Date: Mon, 9 Mar 2026 11:43:03 +0100 Subject: [PATCH 8/9] ci: use yarn install in storage layout workflow --- .github/workflows/storage-layout-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/storage-layout-check.yml b/.github/workflows/storage-layout-check.yml index 5f67fd7..34f3f55 100644 --- a/.github/workflows/storage-layout-check.yml +++ b/.github/workflows/storage-layout-check.yml @@ -19,7 +19,7 @@ jobs: uses: foundry-rs/foundry-toolchain@v1 - name: Install Node dependencies - run: npm ci + run: yarn --frozen-lockfile --network-concurrency 1 - name: Fetch dev branch run: git fetch origin dev:refs/remotes/origin/dev From 044e6000f40926b0d8b1affa6d43fcaa653b7966 Mon Sep 17 00:00:00 2001 From: Francisco Rolotti <102830313+franrolotti@users.noreply.github.com> Date: Mon, 9 Mar 2026 12:19:27 +0100 Subject: [PATCH 9/9] ci: replace storage layout diff check with OZ upgrade safety validation for PRs to dev --- ...out-check.yml => upgrade-safety-check.yml} | 8 +- scripts/check_storage_layout.sh | 97 --------------- scripts/check_upgrade_safety.sh | 116 ++++++++++++++++++ 3 files changed, 120 insertions(+), 101 deletions(-) rename .github/workflows/{storage-layout-check.yml => upgrade-safety-check.yml} (77%) delete mode 100755 scripts/check_storage_layout.sh create mode 100755 scripts/check_upgrade_safety.sh diff --git a/.github/workflows/storage-layout-check.yml b/.github/workflows/upgrade-safety-check.yml similarity index 77% rename from .github/workflows/storage-layout-check.yml rename to .github/workflows/upgrade-safety-check.yml index 34f3f55..031f02f 100644 --- a/.github/workflows/storage-layout-check.yml +++ b/.github/workflows/upgrade-safety-check.yml @@ -1,4 +1,4 @@ -name: Storage Layout Check +name: Upgrade Safety Check on: pull_request: @@ -6,7 +6,7 @@ on: - dev jobs: - storage-layout: + upgrade-safety: runs-on: ubuntu-latest steps: - name: Checkout repository @@ -24,5 +24,5 @@ jobs: - name: Fetch dev branch run: git fetch origin dev:refs/remotes/origin/dev - - name: Check SavingCircles storage layout - run: ./scripts/check_storage_layout.sh origin/dev HEAD src/contracts/SavingCircles.sol:SavingCircles + - name: Check SavingCircles upgrade safety + run: ./scripts/check_upgrade_safety.sh origin/dev HEAD src/contracts/SavingCircles.sol:SavingCircles diff --git a/scripts/check_storage_layout.sh b/scripts/check_storage_layout.sh deleted file mode 100755 index 184cfb8..0000000 --- a/scripts/check_storage_layout.sh +++ /dev/null @@ -1,97 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -OLD_REF="${1:-dev}" -NEW_REF="${2:-HEAD}" -CONTRACT_TARGET="${3:-src/contracts/SavingCircles.sol:SavingCircles}" - -ROOT_DIR="$(git rev-parse --show-toplevel)" -TMP_DIR="$(mktemp -d)" -OLD_DIR="$TMP_DIR/old" -NEW_DIR="$TMP_DIR/new" - -cleanup() { - git -C "$ROOT_DIR" worktree remove "$OLD_DIR" --force >/dev/null 2>&1 || true - git -C "$ROOT_DIR" worktree remove "$NEW_DIR" --force >/dev/null 2>&1 || true - rm -rf "$TMP_DIR" -} -trap cleanup EXIT - -link_deps() { - local target_dir="$1" - if [[ -d "$ROOT_DIR/node_modules" && ! -e "$target_dir/node_modules" ]]; then - ln -s "$ROOT_DIR/node_modules" "$target_dir/node_modules" - fi - if [[ -d "$ROOT_DIR/lib" && ! -e "$target_dir/lib" ]]; then - ln -s "$ROOT_DIR/lib" "$target_dir/lib" - fi -} - -inspect_layout() { - local workdir="$1" - local output_file="$2" - local raw_file="$TMP_DIR/raw-$(basename "$output_file").txt" - local canonical_file="$TMP_DIR/canonical-$(basename "$output_file")" - - if ! (cd "$workdir" && forge inspect "$CONTRACT_TARGET" storage-layout --json > "$raw_file"); then - echo "forge inspect failed in $workdir" - cat "$raw_file" - return 1 - fi - - if ! jq -S . < "$raw_file" > "$output_file"; then - echo "Failed to parse forge inspect output as JSON in $workdir" - cat "$raw_file" - return 1 - fi - - # Canonicalize layout by dropping compiler-internal IDs and normalizing type IDs. - if ! jq -S ' - def norm_type: - gsub("t_struct\\(([^)]*)\\)[0-9]+_storage"; "t_struct(\\1)_storage"); - - { - storage: [.storage[] | {label, slot, offset, type: (.type | norm_type)}], - structs: ( - .types - | to_entries - | map(.value) - | map(select(.label | startswith("struct "))) - | map({ - label, - members: [.members[] | {label, slot, offset, type: (.type | norm_type)}] - }) - | sort_by(.label) - ) - } - ' "$output_file" > "$canonical_file"; then - echo "Failed to canonicalize storage layout in $workdir" - cat "$output_file" - return 1 - fi -} - -echo "Comparing storage layout for $CONTRACT_TARGET" -echo "Old ref: $OLD_REF" -echo "New ref: $NEW_REF" - -git -C "$ROOT_DIR" worktree add --detach "$OLD_DIR" "$OLD_REF" >/dev/null -link_deps "$OLD_DIR" -inspect_layout "$OLD_DIR" "$TMP_DIR/old-layout.json" - -if [[ "$NEW_REF" == "HEAD" ]]; then - inspect_layout "$ROOT_DIR" "$TMP_DIR/new-layout.json" -else - git -C "$ROOT_DIR" worktree add --detach "$NEW_DIR" "$NEW_REF" >/dev/null - link_deps "$NEW_DIR" - inspect_layout "$NEW_DIR" "$TMP_DIR/new-layout.json" -fi - -if diff -u \ - "$TMP_DIR/canonical-old-layout.json" \ - "$TMP_DIR/canonical-new-layout.json"; then - echo "Storage layout check passed: no differences detected." -else - echo "Storage layout check failed: differences detected." - exit 1 -fi diff --git a/scripts/check_upgrade_safety.sh b/scripts/check_upgrade_safety.sh new file mode 100755 index 0000000..e917e6a --- /dev/null +++ b/scripts/check_upgrade_safety.sh @@ -0,0 +1,116 @@ +#!/usr/bin/env bash +set -euo pipefail + +OLD_REF="${1:-origin/dev}" +NEW_REF="${2:-HEAD}" +CONTRACT_TARGET="${3:-src/contracts/SavingCircles.sol:SavingCircles}" + +ROOT_DIR="$(git rev-parse --show-toplevel)" +TMP_DIR="$(mktemp -d)" +OLD_DIR="$TMP_DIR/old" +NEW_DIR="$TMP_DIR/new" + +cleanup() { + git -C "$ROOT_DIR" worktree remove "$OLD_DIR" --force >/dev/null 2>&1 || true + git -C "$ROOT_DIR" worktree remove "$NEW_DIR" --force >/dev/null 2>&1 || true + rm -rf "$TMP_DIR" +} +trap cleanup EXIT + +require_cmd() { + local cmd="$1" + if ! command -v "$cmd" >/dev/null 2>&1; then + echo "Missing required command: $cmd" + exit 1 + fi +} + +ensure_node_modules_link() { + local target_dir="$1" + if [[ -d "$ROOT_DIR/node_modules" && ! -e "$target_dir/node_modules" ]]; then + ln -s "$ROOT_DIR/node_modules" "$target_dir/node_modules" + fi +} + +prepare_worktree() { + local ref="$1" + local dir="$2" + + git -C "$ROOT_DIR" worktree add --detach "$dir" "$ref" >/dev/null + + if [[ -f "$dir/.gitmodules" ]]; then + git -C "$dir" submodule sync --recursive >/dev/null + git -C "$dir" submodule update --init --recursive >/dev/null + fi + + ensure_node_modules_link "$dir" +} + +build_info_dir_for() { + local workdir="$1" + local out_dir + + out_dir="$(cd "$workdir" && forge config --json | jq -r '.out // "out"')" + echo "$workdir/$out_dir/build-info" +} + +compile_for_validation() { + local workdir="$1" + local label="$2" + + echo "Compiling $label at $workdir ..." + ( + cd "$workdir" + forge clean + forge build --build-info --extra-output storageLayout + ) >/dev/null + + local build_info_dir + build_info_dir="$(build_info_dir_for "$workdir")" + + if [[ ! -d "$build_info_dir" ]]; then + echo "Build info directory not found for $label: $build_info_dir" + echo "Make sure foundry.toml enables build_info." + exit 1 + fi + + if ! find "$build_info_dir" -type f -name '*.json' | grep -q .; then + echo "No build info JSON files found for $label in $build_info_dir" + exit 1 + fi +} + +echo "Running OpenZeppelin upgrade safety validation for $CONTRACT_TARGET" +echo "Old ref: $OLD_REF" +echo "New ref: $NEW_REF" + +require_cmd git +require_cmd jq +require_cmd forge +require_cmd npx + +prepare_worktree "$OLD_REF" "$OLD_DIR" +compile_for_validation "$OLD_DIR" "$OLD_REF" + +if [[ "$NEW_REF" == "HEAD" ]]; then + compile_for_validation "$ROOT_DIR" "$NEW_REF" + NEW_BUILD_INFO_DIR="$(build_info_dir_for "$ROOT_DIR")" +else + prepare_worktree "$NEW_REF" "$NEW_DIR" + compile_for_validation "$NEW_DIR" "$NEW_REF" + NEW_BUILD_INFO_DIR="$(build_info_dir_for "$NEW_DIR")" +fi + +OLD_BUILD_INFO_DIR="$(build_info_dir_for "$OLD_DIR")" + +OLD_REF_BUILD_INFO_LINK="$TMP_DIR/old-build-info" +ln -s "$OLD_BUILD_INFO_DIR" "$OLD_REF_BUILD_INFO_LINK" + +echo "Validating upgrade safety with @openzeppelin/upgrades-core ..." +OZ_UPGRADES_SILENCE_WARNINGS=true npx --yes @openzeppelin/upgrades-core validate "$NEW_BUILD_INFO_DIR" \ + --contract "$CONTRACT_TARGET" \ + --reference "old-build-info:$CONTRACT_TARGET" \ + --referenceBuildInfoDirs "$OLD_REF_BUILD_INFO_LINK" \ + --requireReference + +echo "Upgrade safety check passed for $CONTRACT_TARGET"