From 4678b4dd14cbec4726703c5cc62879aba8b4a63b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammed=20Tanr=C4=B1kulu?= Date: Tue, 12 Nov 2024 15:15:44 +0100 Subject: [PATCH 01/10] implement diamond storage pattern --- README.md | 6 ++-- src/TransparentVerifiableProxy.sol | 45 ++++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 2770e25..5618f66 100644 --- a/README.md +++ b/README.md @@ -12,9 +12,9 @@ A system for deploying and verifying proxy contracts with predictable storage la ### 2. TransparentVerifiableProxy - Transparent proxy pattern with verified storage layout -- Fixed storage slots: - - Slot 0: `salt` (uint256) - - Slot 1: `owner` (address) +- Fixed storage slots via Diamond Storage pattern: + - Slot 'proxy.verifiable.salt': `salt` (uint256) + - Slot 'proxy.verifiable.owner': `owner` (address) - Immutable `creator` field (set in bytecode) - Implements secure upgrade mechanism - Initializable to prevent implementation tampering diff --git a/src/TransparentVerifiableProxy.sol b/src/TransparentVerifiableProxy.sol index 08174af..d19de15 100644 --- a/src/TransparentVerifiableProxy.sol +++ b/src/TransparentVerifiableProxy.sol @@ -9,6 +9,21 @@ import {Proxy} from "@openzeppelin/contracts/proxy/Proxy.sol"; import {ERC1967Utils} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Utils.sol"; import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol"; +// EIP-2535 Diamond Storage pattern +// ref: https://eips.ethereum.org/EIPS/eip-2535#storage +library StorageSlot { + bytes32 constant SLOT_SALT = keccak256("proxy.verifiable.salt"); + bytes32 constant SLOT_OWNER = keccak256("proxy.verifiable.owner"); + + function getSaltSlot() internal pure returns (bytes32) { + return SLOT_SALT; + } + + function getOwnerSlot() internal pure returns (bytes32) { + return SLOT_OWNER; + } +} + interface ITransparentVerifiableProxy { /// @dev See {UUPSUpgradeable-upgradeToAndCall} function upgradeToAndCall(address newImplementation, bytes calldata data) external payable; @@ -18,10 +33,6 @@ contract TransparentVerifiableProxy is Proxy, Initializable { // immutable variable (in bytecode) address public immutable creator; - // storage variables (in storage slots) - uint256 public salt; // Salt, being used creating the proxy (slot 0) - address public owner; // The owner of the proxy contract (slot 1) - // ### EVENTS error ProxyDeniedOwnerAccess(); @@ -52,12 +63,34 @@ contract TransparentVerifiableProxy is Proxy, Initializable { { require(implementation != address(0), "New implementation cannot be the zero address"); - salt = _salt; - owner = _owner; + bytes32 saltSlot = StorageSlot.getSaltSlot(); + bytes32 ownerSlot = StorageSlot.getOwnerSlot(); + assembly { + sstore(saltSlot, _salt) + sstore(ownerSlot, _owner) + } ERC1967Utils.upgradeToAndCall(implementation, data); } + function salt() public view returns (uint256) { + bytes32 slot = StorageSlot.getSaltSlot(); + uint256 value; + assembly { + value := sload(slot) + } + return value; + } + + function owner() public view returns (address) { + bytes32 slot = StorageSlot.getOwnerSlot(); + address value; + assembly { + value := sload(slot) + } + return value; + } + /** * @dev Returns the current implementation address. * From 195f151a7ce7ea5ff7aee0fdc038f6b97b1144f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammed=20Tanr=C4=B1kulu?= Date: Tue, 19 Nov 2024 14:42:09 +0100 Subject: [PATCH 02/10] test storage persistence after registry upgrade --- src/VerifiableFactory.sol | 38 ++++++++++++++++++--------------- test/VerifiableFactory.t.sol | 41 ++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 17 deletions(-) diff --git a/src/VerifiableFactory.sol b/src/VerifiableFactory.sol index d21f805..4c12dba 100644 --- a/src/VerifiableFactory.sol +++ b/src/VerifiableFactory.sol @@ -36,8 +36,6 @@ contract VerifiableFactory { * @return proxy The address of the deployed `TransparentVerifiableProxy`. */ function deployProxy(address implementation, uint256 salt) external returns (address) { - console.log("deploys"); - console.logAddress(msg.sender); bytes32 outerSalt = keccak256(abi.encode(msg.sender, salt)); TransparentVerifiableProxy proxy = new TransparentVerifiableProxy{salt: outerSalt}(address(this)); @@ -74,26 +72,32 @@ contract VerifiableFactory { return false; } try IProxy(proxy).salt() returns (uint256 salt) { - try IProxy(proxy).creator() returns (address creator) { - // verify the creator matches this factory - if (address(this) != creator) { - return false; - } + try IProxy(proxy).owner() returns (address owner) { + try IProxy(proxy).creator() returns (address creator) { + return _verifyContract(proxy, creator, owner, salt); + } catch {} + } catch {} + } catch {} + + return false; + } - // reconstruct the address using CREATE2 and verify it matches - bytes32 outerSalt = keccak256(abi.encode(msg.sender, salt)); + function _verifyContract(address proxy, address creator, address owner, uint256 salt) private view returns (bool) { + // verify the creator matches this factory + if (address(this) != creator) { + return false; + } - // get creation bytecode with constructor arguments - bytes memory bytecode = - abi.encodePacked(type(TransparentVerifiableProxy).creationCode, abi.encode(address(this))); + // reconstruct the address using CREATE2 and verify it matches + bytes32 outerSalt = keccak256(abi.encode(owner, salt)); - address expectedProxyAddress = Create2.computeAddress(outerSalt, keccak256(bytecode), address(this)); + // get creation bytecode with constructor arguments + bytes memory bytecode = + abi.encodePacked(type(TransparentVerifiableProxy).creationCode, abi.encode(address(this))); - return expectedProxyAddress == proxy; - } catch {} - } catch {} + address expectedProxyAddress = Create2.computeAddress(outerSalt, keccak256(bytecode), address(this)); - return false; + return expectedProxyAddress == proxy; } function isContract(address account) internal view returns (bool) { diff --git a/test/VerifiableFactory.t.sol b/test/VerifiableFactory.t.sol index e1b17f4..8f1b02c 100644 --- a/test/VerifiableFactory.t.sol +++ b/test/VerifiableFactory.t.sol @@ -142,6 +142,47 @@ contract VerifiableFactoryTest is Test { assertEq(proxy.creator(), address(factory), "Wrong creator"); } + function test_StoragePersistenceAfterUpgrade() public { + uint256 salt = 1; + address testAccount = makeAddr("testAccount"); + + // deploy proxy + vm.prank(owner); + address proxyAddress = factory.deployProxy(address(implementation), salt); + + // initialize v1 implementation + MockRegistry proxyV1 = MockRegistry(proxyAddress); + + // initialize registry + vm.prank(owner); + proxyV1.initialize(owner); + assertEq(proxyV1.admin(), owner, "Admin should be set"); + + // register an address + vm.prank(owner); + proxyV1.register(testAccount); + assertTrue(proxyV1.registeredAddresses(testAccount), "Address should be registered in V1"); + assertEq(proxyV1.getRegistryVersion(), 1, "Should be V1 implementation"); + + // upgrade to v2 + vm.prank(owner); + factory.upgradeImplementation(proxyAddress, address(implementationV2), ""); + + // verify state persists after upgrade + MockRegistryV2 proxyV2 = MockRegistryV2(proxyAddress); + + // check storage persistence + assertTrue(proxyV2.registeredAddresses(testAccount), "Address registration should persist after upgrade"); + assertEq(proxyV2.admin(), owner, "Admin should persist after upgrade"); + assertEq(proxyV2.getRegistryVersion(), 2, "Should be V2 implementation"); + + // verify v2 functionality still works as it should be + address newTestAccount = makeAddr("newTestAccount"); + vm.prank(owner); + proxyV2.register(newTestAccount); + assertTrue(proxyV2.registeredAddresses(newTestAccount), "Should be able to register new address in V2"); + } + // ### Helpers function isContract(address account) internal view returns (bool) { uint256 size; From b7d1d1c1f9cd85e0a5654f44a8b64a48c140cc36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammed=20Tanr=C4=B1kulu?= Date: Thu, 21 Nov 2024 14:59:50 +0100 Subject: [PATCH 03/10] remove unnecessary creator check, since it's a part of address creation under CREATE2 --- src/VerifiableFactory.sol | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/VerifiableFactory.sol b/src/VerifiableFactory.sol index 4c12dba..bb9d96e 100644 --- a/src/VerifiableFactory.sol +++ b/src/VerifiableFactory.sol @@ -9,8 +9,6 @@ interface IProxy { function salt() external view returns (uint256); function owner() external view returns (address); - - function creator() external view returns (address); } contract VerifiableFactory { @@ -73,21 +71,14 @@ contract VerifiableFactory { } try IProxy(proxy).salt() returns (uint256 salt) { try IProxy(proxy).owner() returns (address owner) { - try IProxy(proxy).creator() returns (address creator) { - return _verifyContract(proxy, creator, owner, salt); - } catch {} + return _verifyContract(proxy, owner, salt); } catch {} } catch {} return false; } - function _verifyContract(address proxy, address creator, address owner, uint256 salt) private view returns (bool) { - // verify the creator matches this factory - if (address(this) != creator) { - return false; - } - + function _verifyContract(address proxy, address owner, uint256 salt) private view returns (bool) { // reconstruct the address using CREATE2 and verify it matches bytes32 outerSalt = keccak256(abi.encode(owner, salt)); From bfcf334bfad011287be15a0b346ae1c60a125e4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammed=20Tanr=C4=B1kulu?= Date: Thu, 21 Nov 2024 17:06:14 +0100 Subject: [PATCH 04/10] use SlotDerivation for the fixed storage slot --- README.md | 6 +-- lib/forge-std | 2 +- lib/openzeppelin-contracts | 2 +- src/TransparentVerifiableProxy.sol | 64 +++++++++++++++--------------- 4 files changed, 36 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 5618f66..382edf4 100644 --- a/README.md +++ b/README.md @@ -12,9 +12,9 @@ A system for deploying and verifying proxy contracts with predictable storage la ### 2. TransparentVerifiableProxy - Transparent proxy pattern with verified storage layout -- Fixed storage slots via Diamond Storage pattern: - - Slot 'proxy.verifiable.salt': `salt` (uint256) - - Slot 'proxy.verifiable.owner': `owner` (address) +- Fixed storage slots via [SlotDerivation](https://docs.openzeppelin.com/contracts/5.x/api/utils#SlotDerivation) under `proxy.verifiable` namespace + - `salt` (uint256) + - `owner` (address) - Immutable `creator` field (set in bytecode) - Implements secure upgrade mechanism - Initializable to prevent implementation tampering diff --git a/lib/forge-std b/lib/forge-std index 1714bee..2b59872 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 1714bee72e286e73f76e320d110e0eaf5c4e649d +Subproject commit 2b59872eee0b8088ddcade39fe8c041e17bb79c0 diff --git a/lib/openzeppelin-contracts b/lib/openzeppelin-contracts index dbb6104..a277d47 160000 --- a/lib/openzeppelin-contracts +++ b/lib/openzeppelin-contracts @@ -1 +1 @@ -Subproject commit dbb6104ce834628e473d2173bbc9d47f81a9eec3 +Subproject commit a277d472d657dbbcd88a3de5cae0c806130e2df2 diff --git a/src/TransparentVerifiableProxy.sol b/src/TransparentVerifiableProxy.sol index d19de15..787457b 100644 --- a/src/TransparentVerifiableProxy.sol +++ b/src/TransparentVerifiableProxy.sol @@ -8,21 +8,8 @@ pragma solidity ^0.8.20; import {Proxy} from "@openzeppelin/contracts/proxy/Proxy.sol"; import {ERC1967Utils} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Utils.sol"; import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol"; - -// EIP-2535 Diamond Storage pattern -// ref: https://eips.ethereum.org/EIPS/eip-2535#storage -library StorageSlot { - bytes32 constant SLOT_SALT = keccak256("proxy.verifiable.salt"); - bytes32 constant SLOT_OWNER = keccak256("proxy.verifiable.owner"); - - function getSaltSlot() internal pure returns (bytes32) { - return SLOT_SALT; - } - - function getOwnerSlot() internal pure returns (bytes32) { - return SLOT_OWNER; - } -} +import {StorageSlot} from "@openzeppelin/contracts/utils/StorageSlot.sol"; +import {SlotDerivation} from "@openzeppelin/contracts/utils/SlotDerivation.sol"; interface ITransparentVerifiableProxy { /// @dev See {UUPSUpgradeable-upgradeToAndCall} @@ -30,6 +17,14 @@ interface ITransparentVerifiableProxy { } contract TransparentVerifiableProxy is Proxy, Initializable { + using StorageSlot for bytes32; + using SlotDerivation for bytes32; + using SlotDerivation for string; + + string internal constant _VERIFICATION_SLOT = "proxy.verifiable"; + string internal constant _SALT = "salt"; + string internal constant _OWNER = "owner"; + // immutable variable (in bytecode) address public immutable creator; @@ -63,32 +58,19 @@ contract TransparentVerifiableProxy is Proxy, Initializable { { require(implementation != address(0), "New implementation cannot be the zero address"); - bytes32 saltSlot = StorageSlot.getSaltSlot(); - bytes32 ownerSlot = StorageSlot.getOwnerSlot(); + bytes32 baseSlot = _VERIFICATION_SLOT.erc7201Slot(); + _setSalt(baseSlot, _salt); + _setOwner(baseSlot, _owner); - assembly { - sstore(saltSlot, _salt) - sstore(ownerSlot, _owner) - } ERC1967Utils.upgradeToAndCall(implementation, data); } function salt() public view returns (uint256) { - bytes32 slot = StorageSlot.getSaltSlot(); - uint256 value; - assembly { - value := sload(slot) - } - return value; + return _getSalt(_VERIFICATION_SLOT.erc7201Slot()); } function owner() public view returns (address) { - bytes32 slot = StorageSlot.getOwnerSlot(); - address value; - assembly { - value := sload(slot) - } - return value; + return _getOwner(_VERIFICATION_SLOT.erc7201Slot()); } /** @@ -129,5 +111,21 @@ contract TransparentVerifiableProxy is Proxy, Initializable { ERC1967Utils.upgradeToAndCall(newImplementation, data); } + function _getSalt(bytes32 baseSlot) internal view returns (uint256) { + return baseSlot.deriveMapping(_SALT).getUint256Slot().value; + } + + function _setSalt(bytes32 baseSlot, uint256 _salt) internal { + baseSlot.deriveMapping(_SALT).getUint256Slot().value = _salt; + } + + function _getOwner(bytes32 baseSlot) internal view returns (address) { + return baseSlot.deriveMapping(_OWNER).getAddressSlot().value; + } + + function _setOwner(bytes32 baseSlot, address _owner) internal { + baseSlot.deriveMapping(_OWNER).getAddressSlot().value = _owner; + } + receive() external payable {} } From ea98561f61440105795c1ea420f1339f71626b16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammed=20Tanr=C4=B1kulu?= Date: Thu, 21 Nov 2024 18:40:53 +0100 Subject: [PATCH 05/10] implement common ITransParentVerifiableProxy interface --- src/ITransparentVerifiableProxy.sol | 10 ++++++++++ src/TransparentVerifiableProxy.sol | 3 ++- src/VerifiableFactory.sol | 13 ++++--------- 3 files changed, 16 insertions(+), 10 deletions(-) create mode 100644 src/ITransparentVerifiableProxy.sol diff --git a/src/ITransparentVerifiableProxy.sol b/src/ITransparentVerifiableProxy.sol new file mode 100644 index 0000000..c5fca5d --- /dev/null +++ b/src/ITransparentVerifiableProxy.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +interface ITransParentVerifiableProxy { + function salt() external view returns (uint256); + + function owner() external view returns (address); + + function creator() external view returns (address); +} diff --git a/src/TransparentVerifiableProxy.sol b/src/TransparentVerifiableProxy.sol index d19de15..0aebffc 100644 --- a/src/TransparentVerifiableProxy.sol +++ b/src/TransparentVerifiableProxy.sol @@ -8,6 +8,7 @@ pragma solidity ^0.8.20; import {Proxy} from "@openzeppelin/contracts/proxy/Proxy.sol"; import {ERC1967Utils} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Utils.sol"; import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol"; +import {ITransParentVerifiableProxy} from "./ITransparentVerifiableProxy.sol"; // EIP-2535 Diamond Storage pattern // ref: https://eips.ethereum.org/EIPS/eip-2535#storage @@ -24,7 +25,7 @@ library StorageSlot { } } -interface ITransparentVerifiableProxy { +interface ITransparentVerifiableProxy is ITransParentVerifiableProxy { /// @dev See {UUPSUpgradeable-upgradeToAndCall} function upgradeToAndCall(address newImplementation, bytes calldata data) external payable; } diff --git a/src/VerifiableFactory.sol b/src/VerifiableFactory.sol index bb9d96e..cfd427d 100644 --- a/src/VerifiableFactory.sol +++ b/src/VerifiableFactory.sol @@ -4,12 +4,7 @@ pragma solidity ^0.8.20; import {console} from "forge-std/console.sol"; import {Create2} from "@openzeppelin/contracts/utils/Create2.sol"; import {ITransparentVerifiableProxy, TransparentVerifiableProxy} from "./TransparentVerifiableProxy.sol"; - -interface IProxy { - function salt() external view returns (uint256); - - function owner() external view returns (address); -} +import {ITransParentVerifiableProxy} from "./ITransParentVerifiableProxy.sol"; contract VerifiableFactory { event ProxyDeployed(address indexed sender, address indexed proxyAddress, uint256 salt, address implementation); @@ -48,7 +43,7 @@ contract VerifiableFactory { // Function to upgrade the proxy's implementation (only owner of proxy can call this) function upgradeImplementation(address proxyAddress, address newImplementation, bytes memory data) external { - address owner = IProxy(proxyAddress).owner(); + address owner = ITransparentVerifiableProxy(proxyAddress).owner(); require(owner == msg.sender, "Only the owner can upgrade"); // Upgrade the proxy to point to the new implementation @@ -69,8 +64,8 @@ contract VerifiableFactory { if (!isContract(proxy)) { return false; } - try IProxy(proxy).salt() returns (uint256 salt) { - try IProxy(proxy).owner() returns (address owner) { + try ITransparentVerifiableProxy(proxy).salt() returns (uint256 salt) { + try ITransparentVerifiableProxy(proxy).owner() returns (address owner) { return _verifyContract(proxy, owner, salt); } catch {} } catch {} From aa39c67419af96251cf5eafbf6d316ae0f044481 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammed=20Tanr=C4=B1kulu?= Date: Thu, 21 Nov 2024 18:49:54 +0100 Subject: [PATCH 06/10] fix interface --- src/ITransparentVerifiableProxy.sol | 5 ++++- src/TransparentVerifiableProxy.sol | 7 +------ src/VerifiableFactory.sol | 4 ++-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/ITransparentVerifiableProxy.sol b/src/ITransparentVerifiableProxy.sol index c5fca5d..f48345a 100644 --- a/src/ITransparentVerifiableProxy.sol +++ b/src/ITransparentVerifiableProxy.sol @@ -1,10 +1,13 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -interface ITransParentVerifiableProxy { +interface ITransparentVerifiableProxy { function salt() external view returns (uint256); function owner() external view returns (address); function creator() external view returns (address); + + /// @dev See {UUPSUpgradeable-upgradeToAndCall} + function upgradeToAndCall(address newImplementation, bytes calldata data) external payable; } diff --git a/src/TransparentVerifiableProxy.sol b/src/TransparentVerifiableProxy.sol index 2b9c1c0..37fd815 100644 --- a/src/TransparentVerifiableProxy.sol +++ b/src/TransparentVerifiableProxy.sol @@ -10,12 +10,7 @@ import {ERC1967Utils} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Utils.s import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol"; import {StorageSlot} from "@openzeppelin/contracts/utils/StorageSlot.sol"; import {SlotDerivation} from "@openzeppelin/contracts/utils/SlotDerivation.sol"; -import {ITransParentVerifiableProxy} from "./ITransparentVerifiableProxy.sol"; - -interface ITransparentVerifiableProxy is ITransParentVerifiableProxy { - /// @dev See {UUPSUpgradeable-upgradeToAndCall} - function upgradeToAndCall(address newImplementation, bytes calldata data) external payable; -} +import {ITransparentVerifiableProxy} from "./ITransparentVerifiableProxy.sol"; contract TransparentVerifiableProxy is Proxy, Initializable { using StorageSlot for bytes32; diff --git a/src/VerifiableFactory.sol b/src/VerifiableFactory.sol index cfd427d..7f05e0a 100644 --- a/src/VerifiableFactory.sol +++ b/src/VerifiableFactory.sol @@ -3,8 +3,8 @@ pragma solidity ^0.8.20; import {console} from "forge-std/console.sol"; import {Create2} from "@openzeppelin/contracts/utils/Create2.sol"; -import {ITransparentVerifiableProxy, TransparentVerifiableProxy} from "./TransparentVerifiableProxy.sol"; -import {ITransParentVerifiableProxy} from "./ITransParentVerifiableProxy.sol"; +import {TransparentVerifiableProxy} from "./TransparentVerifiableProxy.sol"; +import {ITransparentVerifiableProxy} from "./ITransparentVerifiableProxy.sol"; contract VerifiableFactory { event ProxyDeployed(address indexed sender, address indexed proxyAddress, uint256 salt, address implementation); From 73242b9ea28656f3341a25e4926b4893cc038620 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammed=20Tanr=C4=B1kulu?= Date: Mon, 25 Nov 2024 14:58:52 +0100 Subject: [PATCH 07/10] add slot derivation test --- src/TransparentVerifiableProxy.sol | 6 +-- test/TransparentVerifiableProxy.t.sol | 72 +++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 test/TransparentVerifiableProxy.t.sol diff --git a/src/TransparentVerifiableProxy.sol b/src/TransparentVerifiableProxy.sol index 37fd815..1406232 100644 --- a/src/TransparentVerifiableProxy.sol +++ b/src/TransparentVerifiableProxy.sol @@ -27,9 +27,9 @@ contract TransparentVerifiableProxy is Proxy, Initializable { // ### EVENTS error ProxyDeniedOwnerAccess(); - // // Modifier that allows only the owner to call certain functions - // modifier onlyOwner() { - // require(msg.sender == owner, "Caller is not the owner"); + // modifier that allows only the creator address to call certain functions + // modifier onlyCreator() { + // require(msg.sender == creator, "Caller is not the creator address"); // _; // } diff --git a/test/TransparentVerifiableProxy.t.sol b/test/TransparentVerifiableProxy.t.sol new file mode 100644 index 0000000..d39f228 --- /dev/null +++ b/test/TransparentVerifiableProxy.t.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import "forge-std/Test.sol"; +import "../src/TransparentVerifiableProxy.sol"; +import {SlotDerivation} from "@openzeppelin/contracts/utils/SlotDerivation.sol"; +import {MockRegistry} from "../src/mock/MockRegistry.sol"; + +contract TransparentVerifiableProxyTest is Test { + using SlotDerivation for bytes32; + + TransparentVerifiableProxy proxy; + + address creator = address(0x1); + address owner = address(0x2); + address implementation = address(new MockRegistry()); + uint256 salt = 12345; + bytes emptyData; + + string internal constant _VERIFICATION_SLOT = "proxy.verifiable"; + string internal constant _SALT = "salt"; + string internal constant _OWNER = "owner"; + + function setUp() public { + proxy = new TransparentVerifiableProxy(creator); + } + + function testInitialize() public { + // initialize the proxy + proxy.initialize(salt, owner, implementation, emptyData); + + // check salt and owner values + assertEq(proxy.salt(), salt, "Salt mismatch"); + assertEq(proxy.owner(), owner, "Owner mismatch"); + } + + function testSaltStorage() public { + // initialize the proxy + proxy.initialize(salt, owner, implementation, emptyData); + + // compute the base slot + bytes32 baseSlot = SlotDerivation.erc7201Slot(_VERIFICATION_SLOT); + + // use SlotDerivation to compute the salt slot + bytes32 saltSlot = baseSlot.deriveMapping(_SALT); + + // directly manipulate the storage for the salt + uint256 newSalt = 54321; + vm.store(address(proxy), saltSlot, bytes32(newSalt)); + + // verify the updated salt + assertEq(proxy.salt(), newSalt, "Salt update failed"); + } + + function testOwnerStorage() public { + // initialize the proxy + proxy.initialize(salt, owner, implementation, emptyData); + + // compute the base slot + bytes32 baseSlot = SlotDerivation.erc7201Slot(_VERIFICATION_SLOT); + + // use SlotDerivation to compute the owner slot + bytes32 ownerSlot = baseSlot.deriveMapping(_OWNER); + + // directly manipulate the storage for the owner + address newOwner = address(0x4); + vm.store(address(proxy), ownerSlot, bytes32(uint256(uint160(newOwner)))); + + // verify the updated owner + assertEq(proxy.owner(), newOwner, "Owner update failed"); + } +} From b238c8f0c5bb8144f0532fa4d7068b251ac876b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammed=20Tanr=C4=B1kulu?= Date: Mon, 3 Feb 2025 14:03:33 +0100 Subject: [PATCH 08/10] remove dead code, update comments --- src/TransparentVerifiableProxy.sol | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/TransparentVerifiableProxy.sol b/src/TransparentVerifiableProxy.sol index 1406232..264323b 100644 --- a/src/TransparentVerifiableProxy.sol +++ b/src/TransparentVerifiableProxy.sol @@ -27,12 +27,6 @@ contract TransparentVerifiableProxy is Proxy, Initializable { // ### EVENTS error ProxyDeniedOwnerAccess(); - // modifier that allows only the creator address to call certain functions - // modifier onlyCreator() { - // require(msg.sender == creator, "Caller is not the creator address"); - // _; - // } - constructor(address _creator) { creator = _creator; } @@ -81,7 +75,7 @@ contract TransparentVerifiableProxy is Proxy, Initializable { } /** - * @dev If caller is the owner, process the call internally, otherwise transparently fallback to the proxy behavior. + * @dev If caller is the cretor, process the call internally, otherwise transparently fallback to the proxy behavior. */ function _fallback() internal virtual override { if (msg.sender == creator) { From 0510b9e4dbb844df0246d9587f5ce931fb25f647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammed=20Tanr=C4=B1kulu?= Date: Mon, 3 Feb 2025 14:04:20 +0100 Subject: [PATCH 09/10] prevent arbitrary initialization calls --- src/TransparentVerifiableProxy.sol | 1 + test/TransparentVerifiableProxy.t.sol | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/TransparentVerifiableProxy.sol b/src/TransparentVerifiableProxy.sol index 264323b..7793970 100644 --- a/src/TransparentVerifiableProxy.sol +++ b/src/TransparentVerifiableProxy.sol @@ -46,6 +46,7 @@ contract TransparentVerifiableProxy is Proxy, Initializable { payable initializer { + require(msg.sender == creator, "Unauthorized initialization"); require(implementation != address(0), "New implementation cannot be the zero address"); bytes32 baseSlot = _VERIFICATION_SLOT.erc7201Slot(); diff --git a/test/TransparentVerifiableProxy.t.sol b/test/TransparentVerifiableProxy.t.sol index d39f228..92cfd99 100644 --- a/test/TransparentVerifiableProxy.t.sol +++ b/test/TransparentVerifiableProxy.t.sol @@ -27,6 +27,7 @@ contract TransparentVerifiableProxyTest is Test { function testInitialize() public { // initialize the proxy + vm.prank(creator); proxy.initialize(salt, owner, implementation, emptyData); // check salt and owner values @@ -36,6 +37,7 @@ contract TransparentVerifiableProxyTest is Test { function testSaltStorage() public { // initialize the proxy + vm.prank(creator); proxy.initialize(salt, owner, implementation, emptyData); // compute the base slot @@ -54,6 +56,7 @@ contract TransparentVerifiableProxyTest is Test { function testOwnerStorage() public { // initialize the proxy + vm.prank(creator); proxy.initialize(salt, owner, implementation, emptyData); // compute the base slot From a3b0d24e84b1b1f277c5e7b7f4d17c77ee13b798 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammed=20Tanr=C4=B1kulu?= Date: Tue, 4 Feb 2025 16:17:12 +0100 Subject: [PATCH 10/10] fix submodule --- .gitmodules | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitmodules b/.gitmodules index 690924b..9296efd 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,3 +4,6 @@ [submodule "lib/openzeppelin-contracts"] path = lib/openzeppelin-contracts url = https://github.com/OpenZeppelin/openzeppelin-contracts +[submodule "lib/openzeppelin-contracts-upgradeable"] + path = lib/openzeppelin-contracts-upgradeable + url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable