diff --git a/contracts/access-control/PermanentOwnable.sol b/contracts/access-control/PermanentOwnable.sol new file mode 100644 index 00000000..e71e3284 --- /dev/null +++ b/contracts/access-control/PermanentOwnable.sol @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.4; + +/** + * @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 PermanentOwnable { + address private immutable _OWNER; + + /** + * @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_) { + require(owner_ != address(0), "PermanentOwnable: zero address can not be the owner"); + + _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 { + require(_OWNER == msg.sender, "PermanentOwnable: caller is not the owner"); + } +} diff --git a/contracts/mock/access-control/PermanentOwnableMock.sol b/contracts/mock/access-control/PermanentOwnableMock.sol new file mode 100644 index 00000000..fb023ea5 --- /dev/null +++ b/contracts/mock/access-control/PermanentOwnableMock.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.4; + +import {PermanentOwnable} from "../../access-control/PermanentOwnable.sol"; + +contract PermanentOwnableMock is PermanentOwnable { + event ValidOwner(); + + constructor(address _owner) PermanentOwnable(_owner) {} + + function onlyOwnerMethod() external onlyOwner { + emit ValidOwner(); + } +} diff --git a/contracts/package.json b/contracts/package.json index 0035c46b..d3ce930b 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -1,6 +1,6 @@ { "name": "@solarity/solidity-lib", - "version": "2.6.12", + "version": "2.6.13", "license": "MIT", "author": "Distributed Lab", "readme": "README.md", diff --git a/contracts/proxy/beacon/ProxyBeacon.sol b/contracts/proxy/beacon/ProxyBeacon.sol index 52896cca..bcadb0ef 100644 --- a/contracts/proxy/beacon/ProxyBeacon.sol +++ b/contracts/proxy/beacon/ProxyBeacon.sol @@ -4,29 +4,22 @@ pragma solidity ^0.8.4; import {IBeacon} from "@openzeppelin/contracts/proxy/beacon/IBeacon.sol"; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; +import {PermanentOwnable} from "../../access-control/PermanentOwnable.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 { +contract ProxyBeacon is IBeacon, PermanentOwnable { using Address for address; - address private immutable _OWNER; + constructor() PermanentOwnable(msg.sender) {} address private _implementation; event Upgraded(address implementation); - modifier onlyOwner() { - require(_OWNER == msg.sender, "ProxyBeacon: not an owner"); - _; - } - - constructor() { - _OWNER = msg.sender; - } - /** * @notice The function to upgrade to implementation contract * @param newImplementation_ the new implementation diff --git a/contracts/proxy/transparent/TransparentProxyUpgrader.sol b/contracts/proxy/transparent/TransparentProxyUpgrader.sol index 274916f7..9e9ec416 100644 --- a/contracts/proxy/transparent/TransparentProxyUpgrader.sol +++ b/contracts/proxy/transparent/TransparentProxyUpgrader.sol @@ -4,24 +4,17 @@ pragma solidity ^0.8.4; import {ITransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; +import {PermanentOwnable} from "../../access-control/PermanentOwnable.sol"; + /** * @notice The proxies module * * This is the lightweight helper contract that may be used as a TransparentProxy admin. */ -contract TransparentProxyUpgrader { +contract TransparentProxyUpgrader is PermanentOwnable { using Address for address; - address private immutable _OWNER; - - modifier onlyOwner() { - require(_OWNER == msg.sender, "TransparentProxyUpgrader: not an owner"); - _; - } - - constructor() { - _OWNER = msg.sender; - } + constructor() PermanentOwnable(msg.sender) {} /** * @notice The function to upgrade the implementation contract diff --git a/package-lock.json b/package-lock.json index 1dc5cf48..996cbecc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@solarity/solidity-lib", - "version": "2.6.12", + "version": "2.6.13", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@solarity/solidity-lib", - "version": "2.6.12", + "version": "2.6.13", "hasInstallScript": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 9acede0c..00466310 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@solarity/solidity-lib", - "version": "2.6.12", + "version": "2.6.13", "license": "MIT", "author": "Distributed Lab", "description": "Solidity Library by Distributed Lab", diff --git a/test/access-control/PermanentOwnable.test.ts b/test/access-control/PermanentOwnable.test.ts new file mode 100644 index 00000000..8e9be596 --- /dev/null +++ b/test/access-control/PermanentOwnable.test.ts @@ -0,0 +1,48 @@ +import { ethers } from "hardhat"; +import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; +import { expect } from "chai"; +import { Reverter } from "@/test/helpers/reverter"; +import { ZERO_ADDR } from "@/scripts/utils/constants"; + +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(ZERO_ADDR)).to.be.revertedWith( + "PermanentOwnable: zero address can not be the owner", + ); + }); + + 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.revertedWith( + "PermanentOwnable: caller is not the owner", + ); + }); + }); +}); diff --git a/test/proxy/beacon/ProxyBeacon.test.ts b/test/proxy/beacon/ProxyBeacon.test.ts index a9c679c1..a1e260ab 100644 --- a/test/proxy/beacon/ProxyBeacon.test.ts +++ b/test/proxy/beacon/ProxyBeacon.test.ts @@ -44,7 +44,7 @@ describe("ProxyBeacon", () => { it("only owner should upgrade", async () => { await expect(proxyBeacon.connect(SECOND).upgradeTo(await token.getAddress())).to.be.revertedWith( - "ProxyBeacon: not an owner", + "PermanentOwnable: caller is not the owner", ); }); }); diff --git a/test/proxy/transparent/TransparentProxyUpgrader.test.ts b/test/proxy/transparent/TransparentProxyUpgrader.test.ts index f74416b5..6b021e42 100644 --- a/test/proxy/transparent/TransparentProxyUpgrader.test.ts +++ b/test/proxy/transparent/TransparentProxyUpgrader.test.ts @@ -40,7 +40,7 @@ describe("TransparentProxyUpgrader", () => { it("only owner should upgrade", async () => { await expect( transparentProxyUpgrader.connect(SECOND).upgrade(await proxy.getAddress(), await proxy.getAddress(), "0x"), - ).to.be.revertedWith("TransparentProxyUpgrader: not an owner"); + ).to.be.revertedWith("PermanentOwnable: caller is not the owner"); }); });