diff --git a/contracts/access/APermanentOwnable.sol b/contracts/access/APermanentOwnable.sol deleted file mode 100644 index f695b4e9..00000000 --- a/contracts/access/APermanentOwnable.sol +++ /dev/null @@ -1,50 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.21; - -/** - * @notice The PermanentOwnable module - * - * Contract module which provides a basic access control mechanism, where there is - * an account (an owner) that can be granted exclusive access to specific functions. - * - * The owner is set to the address provided by the deployer. The ownership cannot be further changed. - * - * This module will make available the modifier `onlyOwner`, which can be applied - * to your functions to restrict their use to the owners. - */ -abstract contract APermanentOwnable { - address private immutable _OWNER; - - error InvalidOwner(); - error UnauthorizedAccount(address account); - - /** - * @dev Throws if called by any account other than the owner. - */ - modifier onlyOwner() { - _onlyOwner(); - _; - } - - /** - * @notice Initializes the contract setting the address provided by the deployer as the owner. - * @param owner_ the address of the permanent owner. - */ - constructor(address owner_) { - if (owner_ == address(0)) revert InvalidOwner(); - - _OWNER = owner_; - } - - /** - * @notice Returns the address of the owner. - * @return the permanent owner. - */ - function owner() public view virtual returns (address) { - return _OWNER; - } - - function _onlyOwner() internal view virtual { - if (_OWNER != msg.sender) revert UnauthorizedAccount(msg.sender); - } -} diff --git a/contracts/contracts-registry/AContractsRegistry.sol b/contracts/contracts-registry/AContractsRegistry.sol index bca3d214..b57b2983 100644 --- a/contracts/contracts-registry/AContractsRegistry.sol +++ b/contracts/contracts-registry/AContractsRegistry.sol @@ -56,7 +56,7 @@ abstract contract AContractsRegistry is Initializable { * @notice The initialization function */ function __AContractsRegistry_init() internal onlyInitializing { - _proxyUpgrader = new AdminableProxyUpgrader(); + _proxyUpgrader = new AdminableProxyUpgrader(address(this)); } /** diff --git a/contracts/contracts-registry/pools/APoolContractsRegistry.sol b/contracts/contracts-registry/pools/APoolContractsRegistry.sol index 47604424..14432a77 100644 --- a/contracts/contracts-registry/pools/APoolContractsRegistry.sol +++ b/contracts/contracts-registry/pools/APoolContractsRegistry.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.21; import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import {UpgradeableBeacon} from "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; @@ -9,8 +10,6 @@ import {Paginator} from "../../libs/arrays/Paginator.sol"; import {ADependant} from "../../contracts-registry/ADependant.sol"; -import {ProxyBeacon} from "../../proxy/beacon/ProxyBeacon.sol"; - /** * @notice The PoolContractsRegistry module * @@ -31,7 +30,7 @@ abstract contract APoolContractsRegistry is Initializable, ADependant { address internal _contractsRegistry; - mapping(string => ProxyBeacon) private _beacons; + mapping(string => UpgradeableBeacon) private _beacons; mapping(string => EnumerableSet.AddressSet) private _pools; // name => pool error NoMappingExists(string poolName); @@ -134,7 +133,9 @@ abstract contract APoolContractsRegistry is Initializable, ADependant { ) internal virtual { for (uint256 i = 0; i < names_.length; i++) { if (address(_beacons[names_[i]]) == address(0)) { - _beacons[names_[i]] = ProxyBeacon(_deployProxyBeacon()); + _beacons[names_[i]] = UpgradeableBeacon( + _deployProxyBeacon(newImplementations_[i]) + ); } if (_beacons[names_[i]].implementation() != newImplementations_[i]) { @@ -196,7 +197,7 @@ abstract contract APoolContractsRegistry is Initializable, ADependant { * @notice The utility function to deploy a Proxy Beacon contract to be used within the registry * @return the address of a Proxy Beacon */ - function _deployProxyBeacon() internal virtual returns (address) { - return address(new ProxyBeacon()); + function _deployProxyBeacon(address implementation_) internal virtual returns (address) { + return address(new UpgradeableBeacon(implementation_, address(this))); } } diff --git a/contracts/contracts-registry/pools/pool-factory/APoolFactory.sol b/contracts/contracts-registry/pools/pool-factory/APoolFactory.sol index 384c06c6..6ccfefdc 100644 --- a/contracts/contracts-registry/pools/pool-factory/APoolFactory.sol +++ b/contracts/contracts-registry/pools/pool-factory/APoolFactory.sol @@ -2,12 +2,11 @@ pragma solidity ^0.8.22; import {Create2} from "@openzeppelin/contracts/utils/Create2.sol"; +import {BeaconProxy} from "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; import {ADependant} from "../../../contracts-registry/ADependant.sol"; import {APoolContractsRegistry} from "../APoolContractsRegistry.sol"; -import {PublicBeaconProxy} from "../../../proxy/beacon/PublicBeaconProxy.sol"; - /** * @notice The PoolContractsRegistry module * @@ -43,7 +42,7 @@ abstract contract APoolFactory is ADependant { ) internal virtual returns (address) { return address( - new PublicBeaconProxy( + new BeaconProxy( APoolContractsRegistry(poolRegistry_).getProxyBeacon(poolType_), bytes("") ) @@ -61,7 +60,7 @@ abstract contract APoolFactory is ADependant { ) internal virtual returns (address) { return address( - new PublicBeaconProxy{salt: salt_}( + new BeaconProxy{salt: salt_}( APoolContractsRegistry(poolRegistry_).getProxyBeacon(poolType_), bytes("") ) @@ -98,7 +97,7 @@ abstract contract APoolFactory is ADependant { ) internal view virtual returns (address) { bytes32 bytecodeHash = keccak256( abi.encodePacked( - type(PublicBeaconProxy).creationCode, + type(BeaconProxy).creationCode, abi.encode( APoolContractsRegistry(poolRegistry_).getProxyBeacon(poolType_), bytes("") diff --git a/contracts/mock/access/PermanentOwnableMock.sol b/contracts/mock/access/PermanentOwnableMock.sol deleted file mode 100644 index 0226dac1..00000000 --- a/contracts/mock/access/PermanentOwnableMock.sol +++ /dev/null @@ -1,15 +0,0 @@ -// SPDX-License-Identifier: MIT -// solhint-disable -pragma solidity ^0.8.21; - -import {APermanentOwnable} from "../../access/APermanentOwnable.sol"; - -contract PermanentOwnableMock is APermanentOwnable { - event ValidOwner(); - - constructor(address _owner) APermanentOwnable(_owner) {} - - function onlyOwnerMethod() external onlyOwner { - emit ValidOwner(); - } -} diff --git a/contracts/proxy/adminable/AdminableProxyUpgrader.sol b/contracts/proxy/adminable/AdminableProxyUpgrader.sol index fe9b66dd..767ffe6f 100644 --- a/contracts/proxy/adminable/AdminableProxyUpgrader.sol +++ b/contracts/proxy/adminable/AdminableProxyUpgrader.sol @@ -1,7 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.22; -import {APermanentOwnable} from "../../access/APermanentOwnable.sol"; +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; + import {IAdminableProxy} from "./AdminableProxy.sol"; /** @@ -9,8 +10,8 @@ import {IAdminableProxy} from "./AdminableProxy.sol"; * * This is the lightweight helper contract that may be used as a AdminableProxy admin. */ -contract AdminableProxyUpgrader is APermanentOwnable { - constructor() APermanentOwnable(msg.sender) {} +contract AdminableProxyUpgrader is Ownable { + constructor(address initialOwner_) Ownable(initialOwner_) {} /** * @notice The function to upgrade the implementation contract diff --git a/contracts/proxy/beacon/ProxyBeacon.sol b/contracts/proxy/beacon/ProxyBeacon.sol deleted file mode 100644 index 958075f4..00000000 --- a/contracts/proxy/beacon/ProxyBeacon.sol +++ /dev/null @@ -1,42 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.21; - -import {IBeacon} from "@openzeppelin/contracts/proxy/beacon/IBeacon.sol"; - -import {APermanentOwnable} from "../../access/APermanentOwnable.sol"; - -/** - * @notice The proxies module - * - * This is a lightweight utility ProxyBeacon contract that may be used as a beacon that BeaconProxies point to. - */ -contract ProxyBeacon is IBeacon, APermanentOwnable { - constructor() APermanentOwnable(msg.sender) {} - - address private _implementation; - - event Upgraded(address implementation); - - error NewImplementationNotAContract(address newImplementation); - - /** - * @notice The function to upgrade to implementation contract - * @param newImplementation_ the new implementation - */ - function upgradeTo(address newImplementation_) external virtual onlyOwner { - if (newImplementation_.code.length == 0) - revert NewImplementationNotAContract(newImplementation_); - - _implementation = newImplementation_; - - emit Upgraded(newImplementation_); - } - - /** - * @notice The function to get the address of the implementation contract - * @return the address of the implementation contract - */ - function implementation() public view virtual override returns (address) { - return _implementation; - } -} diff --git a/contracts/proxy/beacon/PublicBeaconProxy.sol b/contracts/proxy/beacon/PublicBeaconProxy.sol deleted file mode 100644 index 8a653c2e..00000000 --- a/contracts/proxy/beacon/PublicBeaconProxy.sol +++ /dev/null @@ -1,24 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.22; - -import {BeaconProxy} from "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; - -/** - * @notice The proxies module - * - * The helper BeaconProxy that can be deployed by the factories. - * - * Note that the external `implementation()` function is added to the contract to provide compatibility with - * Etherscan. This means that the implementation contract must not have such a function declared. - */ -contract PublicBeaconProxy is BeaconProxy { - constructor(address beacon_, bytes memory data_) payable BeaconProxy(beacon_, data_) {} - - /** - * @notice The function that returns implementation contract this proxy points to - * @return address the implementation address - */ - function implementation() public view virtual returns (address) { - return _implementation(); - } -} diff --git a/package-lock.json b/package-lock.json index 7869df13..31f07496 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@solarity/solidity-lib", - "version": "3.0.0-rc.1", + "version": "3.0.0-rc.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@solarity/solidity-lib", - "version": "3.0.0-rc.1", + "version": "3.0.0-rc.2", "license": "MIT", "dependencies": { "@openzeppelin/contracts": "5.2.0", diff --git a/package.json b/package.json index 0fabf426..68c5f81c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@solarity/solidity-lib", - "version": "3.0.0-rc.1", + "version": "3.0.0-rc.2", "license": "MIT", "author": "Distributed Lab", "readme": "README.md", diff --git a/test/access/PermanentOwnable.test.ts b/test/access/PermanentOwnable.test.ts deleted file mode 100644 index f4f75b15..00000000 --- a/test/access/PermanentOwnable.test.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { ethers } from "hardhat"; -import { expect } from "chai"; - -import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; - -import { Reverter } from "@/test/helpers/reverter"; - -import { PermanentOwnableMock } from "@ethers-v6"; - -describe("PermanentOwnable", () => { - const reverter = new Reverter(); - - let OWNER: SignerWithAddress; - let OTHER: SignerWithAddress; - - let permanentOwnable: PermanentOwnableMock; - - before("setup", async () => { - [OWNER, OTHER] = await ethers.getSigners(); - - const permanentOwnableMock = await ethers.getContractFactory("PermanentOwnableMock"); - permanentOwnable = await permanentOwnableMock.deploy(OWNER); - - await reverter.snapshot(); - }); - - afterEach(reverter.revert); - - describe("PermanentOwnable", () => { - it("should set the correct owner", async () => { - expect(await permanentOwnable.owner()).to.equal(OWNER.address); - }); - - it("should reject zero address during the owner initialization", async () => { - const permanentOwnableMock = await ethers.getContractFactory("PermanentOwnableMock"); - - await expect(permanentOwnableMock.deploy(ethers.ZeroAddress)) - .to.be.revertedWithCustomError(permanentOwnable, "InvalidOwner") - .withArgs(); - }); - - it("only owner should call this function", async () => { - expect(await permanentOwnable.connect(OWNER).onlyOwnerMethod()).to.emit(permanentOwnable, "ValidOwner"); - await expect(permanentOwnable.connect(OTHER).onlyOwnerMethod()) - .to.be.revertedWithCustomError(permanentOwnable, "UnauthorizedAccount") - .withArgs(OTHER); - }); - }); -}); diff --git a/test/contracts-registry/pools/pool-factory/PoolFactory.test.ts b/test/contracts-registry/pools/pool-factory/PoolFactory.test.ts index 42ecf4c8..9cca3fdb 100644 --- a/test/contracts-registry/pools/pool-factory/PoolFactory.test.ts +++ b/test/contracts-registry/pools/pool-factory/PoolFactory.test.ts @@ -7,7 +7,7 @@ import { Reverter } from "@/test/helpers/reverter"; import { PoolFactoryMock, - PublicBeaconProxy, + BeaconProxy, PoolContractsRegistryMock, ContractsRegistryPoolMock, PoolMock, @@ -97,12 +97,11 @@ describe("PoolFactory", () => { expect(await poolContractsRegistry.countPools(NAME_2)).to.equal(0n); const PoolMock = await ethers.getContractFactory("PoolMock"); - const PublicBeaconProxy = await ethers.getContractFactory("PublicBeaconProxy"); + const BeaconProxy = await ethers.getContractFactory("BeaconProxy"); const pool = PoolMock.attach((await poolContractsRegistry.listPools(NAME_1, 0, 1))[0]); - const beaconProxy = PublicBeaconProxy.attach(await pool.getAddress()); + const beaconProxy = BeaconProxy.attach(await pool.getAddress()); - expect(await beaconProxy.implementation()).to.equal(await poolImpl.getAddress()); expect(await pool.token()).not.to.equal(ethers.ZeroAddress); }); @@ -177,20 +176,12 @@ describe("PoolFactory", () => { expect(pools).to.deep.equal([predictedAddress1, predictedAddress2]); const PoolMock = await ethers.getContractFactory("PoolMock"); - const PublicBeaconProxy = await ethers.getContractFactory("PublicBeaconProxy"); const poolProxies = await Promise.all(pools.map(async (pool: string) => PoolMock.attach(pool))); - const beaconProxies = await Promise.all( - pools.map(async (pool: string) => PublicBeaconProxy.attach(pool)), - ); const tokens = await Promise.all(poolProxies.map(async (poolProxy: PoolMock) => await poolProxy.token())); - const implementations = await Promise.all( - beaconProxies.map(async (beaconProxy: PublicBeaconProxy) => await beaconProxy.implementation()), - ); expect(tokens).to.deep.equal([await token.getAddress(), await token.getAddress()]); - expect(implementations).to.deep.equal([await poolImpl.getAddress(), await poolImpl.getAddress()]); }); it("should revert when deploying the pool with the same salt", async () => { diff --git a/test/proxy/beacon/ProxyBeacon.test.ts b/test/proxy/beacon/ProxyBeacon.test.ts deleted file mode 100644 index addf8f8b..00000000 --- a/test/proxy/beacon/ProxyBeacon.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { ethers } from "hardhat"; -import { expect } from "chai"; - -import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; - -import { Reverter } from "@/test/helpers/reverter"; - -import { ProxyBeacon, ERC20Mock } from "@ethers-v6"; - -describe("ProxyBeacon", () => { - const reverter = new Reverter(); - - let OWNER: SignerWithAddress; - let SECOND: SignerWithAddress; - - let proxyBeacon: ProxyBeacon; - let token: ERC20Mock; - - before("setup", async () => { - [OWNER, SECOND] = await ethers.getSigners(); - - const ProxyBeacon = await ethers.getContractFactory("ProxyBeacon"); - const ERC20Mock = await ethers.getContractFactory("ERC20Mock"); - - proxyBeacon = await ProxyBeacon.deploy(); - token = await ERC20Mock.deploy("mock", "mock", 18); - - await reverter.snapshot(); - }); - - afterEach(reverter.revert); - - describe("functions", () => { - it("should upgrade", async () => { - expect(await proxyBeacon.implementation()).to.equal(ethers.ZeroAddress); - - await proxyBeacon.upgradeTo(await token.getAddress()); - - expect(await proxyBeacon.implementation()).to.equal(await token.getAddress()); - }); - - it("should not upgrade to non-contract", async () => { - await expect(proxyBeacon.upgradeTo(SECOND)) - .to.be.revertedWithCustomError(proxyBeacon, "NewImplementationNotAContract") - .withArgs(SECOND); - }); - - it("only owner should upgrade", async () => { - await expect(proxyBeacon.connect(SECOND).upgradeTo(await token.getAddress())) - .to.be.revertedWithCustomError(proxyBeacon, "UnauthorizedAccount") - .withArgs(SECOND); - }); - }); -}); diff --git a/test/proxy/beacon/PublicBeaconProxy.test.ts b/test/proxy/beacon/PublicBeaconProxy.test.ts deleted file mode 100644 index 701197df..00000000 --- a/test/proxy/beacon/PublicBeaconProxy.test.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { ethers } from "hardhat"; -import { expect } from "chai"; -import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; - -import { Reverter } from "@/test/helpers/reverter"; - -import { PublicBeaconProxy, ProxyBeacon, ERC20Mock } from "@ethers-v6"; - -describe("ProxyBeacon", () => { - const reverter = new Reverter(); - - let OWNER: SignerWithAddress; - - let proxyBeacon: ProxyBeacon; - let token: ERC20Mock; - - before("setup", async () => { - [OWNER] = await ethers.getSigners(); - - const ProxyBeacon = await ethers.getContractFactory("ProxyBeacon"); - const ERC20Mock = await ethers.getContractFactory("ERC20Mock"); - - proxyBeacon = await ProxyBeacon.deploy(); - token = await ERC20Mock.deploy("mock", "mock", 18); - - await reverter.snapshot(); - }); - - afterEach(reverter.revert); - - describe("functions", () => { - it("should get implementation", async () => { - await proxyBeacon.upgradeTo(await token.getAddress()); - - const PublicBeaconProxy = await ethers.getContractFactory("PublicBeaconProxy"); - const proxy: PublicBeaconProxy = await PublicBeaconProxy.deploy(proxyBeacon, "0x"); - - expect(await proxyBeacon.implementation()).to.equal(await token.getAddress()); - expect(await proxy.implementation()).to.equal(await token.getAddress()); - }); - }); -}); diff --git a/test/proxy/transparent/AdminableProxy.test.ts b/test/proxy/transparent/AdminableProxy.test.ts index e3112ce0..15a03075 100644 --- a/test/proxy/transparent/AdminableProxy.test.ts +++ b/test/proxy/transparent/AdminableProxy.test.ts @@ -26,7 +26,7 @@ describe("AdminableProxy", () => { const token: ERC20Mock = await ERC20Mock.deploy("mock", "mock", 18); - const adminableProxyUpgrader: AdminableProxyUpgrader = await AdminableProxyUpgrader.deploy(); + const adminableProxyUpgrader: AdminableProxyUpgrader = await AdminableProxyUpgrader.deploy(OWNER); proxy = await AdminableProxy.deploy(await token.getAddress(), await adminableProxyUpgrader.getAddress(), "0x"); tokenProxy = token.attach(await proxy.getAddress()); diff --git a/test/proxy/transparent/AdminableProxyUpgrader.test.ts b/test/proxy/transparent/AdminableProxyUpgrader.test.ts index 9d5e98e1..1a90fade 100644 --- a/test/proxy/transparent/AdminableProxyUpgrader.test.ts +++ b/test/proxy/transparent/AdminableProxyUpgrader.test.ts @@ -25,7 +25,7 @@ describe("AdminableProxyUpgrader", () => { token = await ERC20Mock.deploy("mock", "mock", 18); - adminableProxyUpgrader = await AdminableProxyUpgrader.deploy(); + adminableProxyUpgrader = await AdminableProxyUpgrader.deploy(OWNER); proxy = await AdminableProxy.deploy(await token.getAddress(), await adminableProxyUpgrader.getAddress(), "0x"); await reverter.snapshot(); @@ -38,7 +38,7 @@ describe("AdminableProxyUpgrader", () => { await expect( adminableProxyUpgrader.connect(SECOND).upgrade(await proxy.getAddress(), await proxy.getAddress(), "0x"), ) - .to.be.revertedWithCustomError(adminableProxyUpgrader, "UnauthorizedAccount") + .to.be.revertedWithCustomError(adminableProxyUpgrader, "OwnableUnauthorizedAccount") .withArgs(SECOND); }); });