diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 34a4a52..d2c65c8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -27,11 +27,6 @@ jobs: run: | forge --version - - name: Run Forge fmt - run: | - forge fmt --check - id: fmt - - name: Run Forge build run: | forge build --sizes diff --git a/README.md b/README.md index e7837a1..ea81318 100644 --- a/README.md +++ b/README.md @@ -12,30 +12,28 @@ forge test ``` ## Storage layout of `SemVerProxy` -`SemVerProxy` reserves storage slots **0 to 99** for implementation contracts by declaring a fixed-size array that occupies these slots: -```solidity -uint256[100] private __gap; // Reserves slots 0-99 for implementations -``` -So, the storage layout of a proxy and an arbitrary implementation contract can be represented as follows: +`SemVerProxy` stores variables in the storage, starting at slot 1000. Therefore, implementation contracts can safely use all storage slots except 1000, 1001, and 1002. +So, the storage layout of the proxy and an arbitrary implementation contract can be represented as follows: ```text Proxy storage layout: -┌─────────────┐ -│ Slot 0-99 │ ← `__gap` placeholder (reserved for implementations) -├─────────────┤ -│ Slot 100 │ ← `_latestVersion` -│ Slot 101 │ ← Other state variables -│ ... │ -└─────────────┘ +┌───────────────┐ +│ Slot 0-1000 │ ← empty +├───────────────┤ +│ Slot 1000 │ ← `_latestVersion` +│ Slot 1001 │ ← `_releases` mapping +│ Slot 1002 │ ← `_subscribedClients` mapping +│ ... │ +└───────────────┘ -Implementation storage example: -┌─────────────┐ -│ Slot 0 │ ← Implementation's first state variable -│ Slot 1 │ ← Implementation's second state variable -│ ... │ -│ Slot 99 │ ← Last available slot for implementation -├─────────────┤ -| Slot 100 | ← ⚠️ This will collide with proxy's storage -└─────────────┘ +Implementation's storage example: +┌───────────────┐ +│ Slot 0 │ ← Implementation's 1st state variable +│ Slot 1 │ ← Implementation's 2nd state variable +│ ... │ +│ Slot 999 │ ← Implementation's 998th state variable +├───────────────┤ +| Slot 1000 │ ← ⚠️ This will collide with proxy's storage +└───────────────┘ ``` ## Releases and Versioning @@ -91,5 +89,5 @@ function unsubscribeFromVersioning() external; ``` ## Security Considerations -- Since `SemVerProxy` only reserves slots from 0 to 99, any 100+ slot of the implementation will collide with proxy. +- Storing variables in implementation's storage at slots 1000, 1001, 1002 will collide with the proxy's storage. - `SemVerProxy` has externally accessable function, therefore there's a possibiliy of function selector clash (i.e., if implementation defines functions that have the same signature as external functions of `SemVerProxy`). diff --git a/foundry.toml b/foundry.toml index a4cb957..b10537c 100644 --- a/foundry.toml +++ b/foundry.toml @@ -7,4 +7,6 @@ libs = ["lib"] runs = 1000 depth = 300 +solc_version = "0.8.30" + # See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options diff --git a/src/SemVerProxy.sol b/src/SemVerProxy.sol index 70f1a64..80741ac 100644 --- a/src/SemVerProxy.sol +++ b/src/SemVerProxy.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.28; +pragma solidity 0.8.30; import {TransparentUpgradeableProxy, ERC1967Utils} from "openzeppelin-contracts/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import {Versioning, Version, EncodedVersion} from "./lib/Versioning.sol"; @@ -23,31 +23,22 @@ import {Clients, Client} from "./lib/Clients.sol"; * specified {_fallback} will delegate to the latest release. * * @dev Latest version is stored in a storage slot, specified in ERC-1967. + * + * @dev Storage variables are stored starting from 1000th slot. */ -contract SemVerProxy is TransparentUpgradeableProxy { - /** - * @dev Since {store} and {obtainRelease} are methods - * that should be applied to {mapping(EncodedVersion => address)} - * we limit them exactly to this mapping type. - */ +contract SemVerProxy is TransparentUpgradeableProxy layout at 1_000 { using { Versioning.store, Versioning.obtainRelease } for mapping(EncodedVersion => address); - /** - * @dev Incrementors and {encode} functions are limited - * to be applied to {Version} struct. - */ + using { Versioning.incMajor, Versioning.incMinor, Versioning.incPatch, Versioning.encode } for Version; - /** - * @dev Subscription logic is limited to be - * applied to {mapping(Client => address)} type. - */ + using { Clients.subscribe, Clients.unsubscribe, @@ -60,16 +51,9 @@ contract SemVerProxy is TransparentUpgradeableProxy { event ClientUnsubscribed(Client indexed client); /*** * STORAGE * ***/ - /** - * @dev Reserve 100 storage slots to be used in implementations. - * @notice Any 99+ slot inside implementation will overwrite - * storage of this proxy, - * Therefore, implementations can only safely use - * slots between 0 and 99. - */ - uint256[100] private __gap; /// @notice Stores SemVer-like latest version of the implemenation. + /// @dev {_latestVersion} is stored in storage slot [1_000]. Version internal _latestVersion; /// @notice Stores addresses of every existing version. diff --git a/src/interfaces/ISemVerProxy.sol b/src/interfaces/ISemVerProxy.sol index e7504aa..9ef654a 100644 --- a/src/interfaces/ISemVerProxy.sol +++ b/src/interfaces/ISemVerProxy.sol @@ -4,17 +4,19 @@ pragma solidity ^0.8.28; import {Version, EncodedVersion} from "../lib/Versioning.sol"; interface ISemVerProxy { - function latestVersion() external view returns(Version memory); + function latestVersion() external view returns (Version memory); function latestEncoded() external view returns (EncodedVersion); function latestRelease() external view returns (address); - - /*~~~~~~~~~~~~~~~~~~~~~~ CLIENT ACTIONS ~~~~~~~~~~~~~~~~~~~~~~*/ + /** + * * CLIENT ACTIONS * ** + */ function subscribeToVersion(Version memory version) external; function unsubscribeFromVersioning() external; - /*~~~~~~~~~~~~~~~~~~~~~~ ADMIN ACTIONS ~~~~~~~~~~~~~~~~~~~~~~*/ - + /** + * * ADMIN ACTIONS * ** + */ function releaseMajor(address release, bytes memory data) external; function releaseMinor(address release, bytes memory data) external; function releasePatch(address release, bytes memory data) external; diff --git a/test/SemVerProxy.t.sol b/test/SemVerProxy.t.sol index 997a498..8de8e40 100644 --- a/test/SemVerProxy.t.sol +++ b/test/SemVerProxy.t.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.8.28; +pragma solidity 0.8.30; import {ProxyAdmin} from "openzeppelin-contracts/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import {Versioning, Version, EncodedVersion} from "../src/lib/Versioning.sol"; @@ -49,7 +49,7 @@ contract SemVerProxyTest is Test { // Verify that writing to unsafe slots breaks storage. function test_implementationBreaksStorage() public { - // This contract might write to 100th storage slot. + // This contract might write to 1_000th storage slot. Breaking breaking = new Breaking(); vm.prank(address(proxyAdmin)); @@ -60,27 +60,26 @@ contract SemVerProxyTest is Test { (uint64 major, uint64 minor, uint128 patch) = implementation .latestVersion_(); - // This will succeed, since {Breaking} contract - // will read from a storage slot already occupied - // by the {SemVerProxy}. + // This will succeed, since {Breaking} contract + // will read from a storage slot already occupied + // by the {SemVerProxy}. _compareVersions( Version(major, minor, patch), semVerProxy.latestVersion() ); - implementation.setVersion(); - (major, minor, patch) = implementation - .latestVersion_(); + implementation.setVersion(); + (major, minor, patch) = implementation.latestVersion_(); - // This will succeed again, because at this point - // the storage slot {100} has collided between - // {SemVerProxy} and {Breaking} contract, and - // {setVersion} call has ovrewritten {_latestVersion} - // storage variable of {SemVerProxy}. + // This will succeed again, because at this point + // the storage slot {1_000} has collided between + // {SemVerProxy} and {Breaking} contract, and + // {setVersion} call has ovrewritten {_latestVersion} + // storage variable of {SemVerProxy}. _compareVersions( Version(major, minor, patch), semVerProxy.latestVersion() - ); + ); } function testFuzz_subscribe(address caller) public { @@ -292,7 +291,10 @@ contract SemVerProxyTest is Test { assertEq(implementation.x(), release2.ANOTHA_WILL_BE_X()); } - function _compareVersions(Version memory v0, Version memory v1) internal { + function _compareVersions( + Version memory v0, + Version memory v1 + ) internal pure { assertEq( EncodedVersion.unwrap(v0.encode()), EncodedVersion.unwrap(v1.encode()) diff --git a/test/InvariantSemVerProxy.t.sol b/test/SemVerProxyInv.t.sol similarity index 96% rename from test/InvariantSemVerProxy.t.sol rename to test/SemVerProxyInv.t.sol index 396f751..df961a6 100644 --- a/test/InvariantSemVerProxy.t.sol +++ b/test/SemVerProxyInv.t.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.8.28; +pragma solidity 0.8.30; import {ProxyAdmin} from "openzeppelin-contracts/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import {Versioning, Version, EncodedVersion} from "../src/lib/Versioning.sol"; @@ -7,7 +7,7 @@ import {X, Y, Z} from "./mocks/VersionedContract.sol"; import {SemVerProxy} from "../src/SemVerProxy.sol"; import {Test, Vm} from "forge-std/Test.sol"; -contract InvariantSemVerProxyTest is Test { +contract SemVerProxyInv is Test { using {Versioning.encode} for Version; address latestRelease; diff --git a/test/libs/Clients.t.sol b/test/libs/Clients.t.sol index 50b09b4..35fe483 100644 --- a/test/libs/Clients.t.sol +++ b/test/libs/Clients.t.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.8.28; +pragma solidity 0.8.30; import {Client, Clients} from "../../src/lib/Clients.sol"; import {Test} from "forge-std/Test.sol"; diff --git a/test/libs/Versioning.t.sol b/test/libs/Versioning.t.sol index 5270d74..09b41c3 100644 --- a/test/libs/Versioning.t.sol +++ b/test/libs/Versioning.t.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.8.28; +pragma solidity 0.8.30; import {Version, Versioning, EncodedVersion} from "../../src/lib/Versioning.sol"; import {Test} from "forge-std/Test.sol"; diff --git a/test/mocks/BreakingContract.sol b/test/mocks/BreakingContract.sol index dc1accb..b448487 100644 --- a/test/mocks/BreakingContract.sol +++ b/test/mocks/BreakingContract.sol @@ -1,9 +1,8 @@ -pragma solidity ^0.8.28; +pragma solidity 0.8.30; import {Version} from "../../src/lib/Versioning.sol"; -contract Breaking { - uint256[100] private __gap; +contract Breaking layout at 1_000 { Version public latestVersion_; function setVersion() external { diff --git a/test/mocks/VersionedContract.sol b/test/mocks/VersionedContract.sol index fa32b82..a25ff74 100644 --- a/test/mocks/VersionedContract.sol +++ b/test/mocks/VersionedContract.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.8.28; +pragma solidity 0.8.30; contract X { uint256 public constant WILL_BE_X = 228;