diff --git a/contracts/access/EcoOwnable.sol b/contracts/access/EcoOwnable.sol index e153214..b106439 100644 --- a/contracts/access/EcoOwnable.sol +++ b/contracts/access/EcoOwnable.sol @@ -65,4 +65,10 @@ abstract contract EcoOwnable is override(Ownable2StepUpgradeable) { return super._transferOwnership(newOwner); } +} + +contract TestEcoOwnable is EcoOwnable { + constructor() { + initEcoOwnable(_msgSender()); + } } \ No newline at end of file diff --git a/contracts/access/SelectorRoleControlUpgradeable.sol b/contracts/access/SelectorRoleControlUpgradeable.sol index 6633080..55ba1bf 100644 --- a/contracts/access/SelectorRoleControlUpgradeable.sol +++ b/contracts/access/SelectorRoleControlUpgradeable.sol @@ -7,26 +7,25 @@ import { IEcoOwnable, EcoOwnable } from "./EcoOwnable.sol"; import { AccessControlEnumerableUpgradeable, AccessControlUpgradeable } from "@openzeppelin/contracts-upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol"; import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol"; -import {IAccessControlEnumerable, IAccessControl} from "@openzeppelin/contracts/access/extensions/IAccessControlEnumerable.sol"; +import { IAccessControlEnumerable, IAccessControl } from "@openzeppelin/contracts/access/extensions/IAccessControlEnumerable.sol"; interface IPausable { // function paused() external view returns (bool); already defined & implemented - function pause() external ; - function unpause() external ; + function pause() external; + + function unpause() external; } interface IMulticall { function multicall(bytes[] calldata data) external returns (bytes[] memory results); } -interface ISelectorRoleControl is - // IEcoOwnable, TODO: check redundant override, already defined & implemented - IAccessControlEnumerable, - IPausable -{ - function pause() external ; - function unpause() external ; +// IEcoOwnable, TODO: check redundant override, already defined & implemented +interface ISelectorRoleControl is IAccessControlEnumerable, IPausable { + function pause() external; + + function unpause() external; } contract SelectorRoleControlUpgradeable is @@ -36,7 +35,7 @@ contract SelectorRoleControlUpgradeable is PausableUpgradeable, AccessControlEnumerableUpgradeable { - modifier onlyAdmin { + modifier onlyAdmin() { _onlyAdmin(_msgSender()); _; } @@ -46,14 +45,18 @@ contract SelectorRoleControlUpgradeable is if (owner() != account) _checkRole(msg.sig, account); } - function grantRole(bytes32 role, address account) public virtual onlyAdmin - override(IAccessControl, AccessControlUpgradeable) { - _grantRole(role, account); + function grantRole( + bytes32 role, + address account + ) public virtual override(IAccessControl, AccessControlUpgradeable) onlyAdmin { + require(_grantRole(role, account), "role exist"); } - function revokeRole(bytes32 role, address account) public virtual onlyAdmin - override(IAccessControl, AccessControlUpgradeable) { - _revokeRole(role, account); + function revokeRole( + bytes32 role, + address account + ) public virtual override(IAccessControl, AccessControlUpgradeable) onlyAdmin { + require(_revokeRole(role, account), "role not exist"); } function paused() public view virtual override returns (bool) { @@ -68,4 +71,4 @@ contract SelectorRoleControlUpgradeable is function unpause() public virtual override onlyAdmin { _unpause(); } -} \ No newline at end of file +} diff --git a/hardhat.config.ts b/hardhat.config.ts index 4fb3feb..32f5e01 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -4,6 +4,7 @@ import "@nomicfoundation/hardhat-toolbox"; import "./tasks/utils"; import "hardhat-tracer"; +import "solidity-coverage" const config: HardhatUserConfig = { solidity: { diff --git a/package.json b/package.json index 95a1d02..0b1fe20 100644 --- a/package.json +++ b/package.json @@ -16,28 +16,27 @@ "@typescript-eslint/eslint-plugin": "^5.46.1", "@typescript-eslint/parser": "^5.46.1", "async-constructor": "^0.4.17", + "chai": "^4.2.0", "eslint": "^8.29.0", "eslint-config-prettier": "^8.5.0", "eslint-import-resolver-typescript": "^3.5.2", "eslint-plugin-import": "^2.26.0", "eslint-plugin-prettier": "^4.2.1", - "chai": "^4.2.0", "ethers": "^6.4.0", "hardhat": "^2.19.2", "hardhat-contract-sizer": "^2.10.0", "hardhat-gas-reporter": "^1.0.8", "hardhat-tracer": "^2.7.0", - "solidity-coverage": "^0.8.0", + "prettier": "^2.8.1", + "prettier-eslint": "^15.0.1", + "solidity-coverage": "^0.8.5", "ts-node": ">=8.0.0", "typechain": "^8.3.0", - "typescript": ">=4.5.0", - "prettier": "^2.8.1", - "prettier-eslint": "^15.0.1" + "typescript": ">=4.5.0" }, "dependencies": { "axios": "^1.6.4", "dotenv": "^16.3.1", - "chai": "^4.3.7", "tsconfig-paths": "^4.2.0" } } diff --git a/test/access/EcoOwnable.ts b/test/access/EcoOwnable.ts new file mode 100644 index 0000000..411238b --- /dev/null +++ b/test/access/EcoOwnable.ts @@ -0,0 +1,96 @@ +import { loadFixture } from "@nomicfoundation/hardhat-toolbox/network-helpers"; +import { expect } from "chai"; +import hre from "hardhat"; + +describe("EcoOwnable", function () { + async function fixtureEcoOwnableDeploy() { + const [initialOwner, nextOwner] = await hre.ethers.getSigners(); + + const EcoOwnable = await hre.ethers.getContractFactory("TestEcoOwnable"); + const ecoOwnable = await EcoOwnable.connect(initialOwner).deploy(); + + return { ecoOwnable, initialOwner, nextOwner }; + } + + async function fixtureEcoOwnablePendingRegistered() { + const { ecoOwnable, initialOwner, nextOwner } = await loadFixture(fixtureEcoOwnableDeploy); + + await expect(ecoOwnable.connect(initialOwner).registerPendingOwner(nextOwner)).not.reverted; + + return { ecoOwnable, initialOwner, nextOwner }; + } + + describe("Test on Deployment", function () { + it("check initialOwner", async function () { + const { ecoOwnable, initialOwner } = await loadFixture(fixtureEcoOwnableDeploy); + expect(await ecoOwnable.owner()).equal(initialOwner.address); + expect(await ecoOwnable.pendingOwner()).equal(hre.ethers.ZeroAddress); + + await expect(ecoOwnable.initEcoOwnable(initialOwner)).reverted; + }); + + it("transfer onwer", async function () { + const { ecoOwnable, initialOwner, nextOwner } = await loadFixture(fixtureEcoOwnableDeploy); + + await expect(ecoOwnable.connect(nextOwner).transferOwnership(nextOwner)).reverted; + await expect(ecoOwnable.connect(initialOwner).transferOwnership(nextOwner)).not.reverted; + await expect(ecoOwnable.connect(initialOwner).transferOwnership(nextOwner)).reverted; + }); + + it("register pending onwer", async function () { + const { ecoOwnable, initialOwner, nextOwner } = await loadFixture(fixtureEcoOwnableDeploy); + + await expect(ecoOwnable.connect(nextOwner).transferOwnership(initialOwner)).reverted; + await expect(ecoOwnable.connect(nextOwner).registerPendingOwner(nextOwner)).reverted; + await expect(ecoOwnable.connect(initialOwner).registerPendingOwner(nextOwner)).not.reverted; + + await expect(ecoOwnable.connect(nextOwner).transferOwnership(initialOwner)).reverted; + await expect(ecoOwnable.connect(nextOwner).registerPendingOwner(initialOwner)).reverted; + }); + it("renounce ownership", async function () { + const { ecoOwnable, initialOwner, nextOwner } = await loadFixture(fixtureEcoOwnableDeploy); + + await expect(ecoOwnable.connect(nextOwner).renounceOwnership()).reverted; + await expect(ecoOwnable.connect(initialOwner).renounceOwnership()).not.reverted; + + await expect(ecoOwnable.connect(nextOwner).transferOwnership(initialOwner)).reverted; + await expect(ecoOwnable.connect(initialOwner).transferOwnership(nextOwner)).reverted; + }); + }); + + describe("Test on Registered pendingOwner", function () { + it("check pendingOwner", async function () { + const { ecoOwnable, initialOwner, nextOwner } = await loadFixture(fixtureEcoOwnablePendingRegistered); + expect(await ecoOwnable.owner()).equal(initialOwner.address); + expect(await ecoOwnable.pendingOwner()).equal(nextOwner.address); + }); + + it("claim registered", async function () { + const { ecoOwnable, initialOwner, nextOwner } = await loadFixture(fixtureEcoOwnablePendingRegistered); + + await expect(ecoOwnable.connect(initialOwner).acceptOwnership()).reverted; + await expect(ecoOwnable.connect(nextOwner).acceptOwnership()).not.reverted; + + await expect(ecoOwnable.connect(initialOwner).acceptOwnership()).reverted; + await expect(ecoOwnable.connect(nextOwner).acceptOwnership()).reverted; + }); + + it("renounce ownership", async function () { + // todo redundant check + const { ecoOwnable, initialOwner, nextOwner } = await loadFixture(fixtureEcoOwnablePendingRegistered); + + await expect(ecoOwnable.connect(nextOwner).renounceOwnership()).reverted; + // remove owner & pendingOwner + await expect(ecoOwnable.connect(initialOwner).renounceOwnership()).not.reverted; + + await expect(ecoOwnable.connect(nextOwner).transferOwnership(initialOwner)).reverted; + await expect(ecoOwnable.connect(initialOwner).transferOwnership(nextOwner)).reverted; + + await expect(ecoOwnable.connect(initialOwner).acceptOwnership()).reverted; + await expect(ecoOwnable.connect(nextOwner).acceptOwnership()).reverted; + + await expect(ecoOwnable.connect(initialOwner).registerPendingOwner(initialOwner)).reverted; + await expect(ecoOwnable.connect(nextOwner).registerPendingOwner(nextOwner)).reverted; + }); + }); +}); diff --git a/test/access/SelectorRoleControlUpgradeable.ts b/test/access/SelectorRoleControlUpgradeable.ts new file mode 100644 index 0000000..09c6c27 --- /dev/null +++ b/test/access/SelectorRoleControlUpgradeable.ts @@ -0,0 +1,73 @@ +import { loadFixture } from "@nomicfoundation/hardhat-toolbox/network-helpers"; +import { expect } from "chai"; +import hre from "hardhat"; + +import { getSelector } from "../helper"; + +describe("SelectorRoleControlUpgradeable", function () { + async function fixtureSelectorRoleControlUpgradeableDeploy() { + const [initialOwner, admin] = await hre.ethers.getSigners(); + + const SelectorRoleControlUpgradeable = await hre.ethers.getContractFactory("SelectorRoleControlUpgradeable"); + const role = await SelectorRoleControlUpgradeable.connect(initialOwner).deploy(); + + await expect(role.initEcoOwnable(initialOwner)).not.reverted; + + return { role, initialOwner, admin }; + } + + describe("Test on Deployment", function () { + it("check initialOwner", async function () { + const { role, initialOwner } = await loadFixture(fixtureSelectorRoleControlUpgradeableDeploy); + expect(await role.owner()).equal(initialOwner.address); + expect(await role.pendingOwner()).equal(hre.ethers.ZeroAddress); + }); + + it("owner pause", async function () { + const { role } = await loadFixture(fixtureSelectorRoleControlUpgradeableDeploy); + + await expect(role.unpause()).reverted; + await expect(role.pause()).not.reverted; + await expect(role.pause()).reverted; + await expect(role.unpause()).not.reverted; + }); + + it("owner grant admin & admin try & play role", async function () { + const { role, admin } = await loadFixture(fixtureSelectorRoleControlUpgradeableDeploy); + + await expect(role.connect(admin).pause()).reverted; + await expect(role.connect(admin).unpause()).reverted; + + await expect(role.revokeRole(getSelector(role.pause), admin)).reverted; + await expect(role.grantRole(getSelector(role.pause), admin)).not.reverted; + await expect(role.grantRole(getSelector(role.pause), admin)).reverted; + + await expect(role.connect(admin).pause()).not.reverted; + await expect(role.connect(admin).pause()).reverted; + + await expect(role.revokeRole(getSelector(role.unpause), admin)).reverted; + await expect(role.grantRole(getSelector(role.unpause), admin)).not.reverted; + await expect(role.grantRole(getSelector(role.unpause), admin)).reverted; + + await expect(role.connect(admin).unpause()).not.reverted; + await expect(role.connect(admin).unpause()).reverted; + + // TODO: unpause revoke test? + await expect(role.revokeRole(getSelector(role.pause), admin)).not.reverted; + await expect(role.revokeRole(getSelector(role.pause), admin)).reverted; + + await expect(role.revokeRole(getSelector(role.unpause), admin)).not.reverted; + await expect(role.revokeRole(getSelector(role.unpause), admin)).reverted; + }); + + it("admin try grant & admin try role", async function () { + const { role, admin } = await loadFixture(fixtureSelectorRoleControlUpgradeableDeploy); + + await expect(role.connect(admin).grantRole(getSelector(role.pause), admin)).reverted; + await expect(role.connect(admin).grantRole(getSelector(role.unpause), admin)).reverted; + + await expect(role.connect(admin).revokeRole(getSelector(role.pause), admin)).reverted; + await expect(role.connect(admin).revokeRole(getSelector(role.unpause), admin)).reverted; + }); + }); +}); diff --git a/test/helper.ts b/test/helper.ts new file mode 100644 index 0000000..6d83de7 --- /dev/null +++ b/test/helper.ts @@ -0,0 +1,5 @@ +import hre from "hardhat"; + +export function getSelector(contractMethod: { fragment: { selector: string } }): string { + return hre.ethers.zeroPadBytes(contractMethod.fragment.selector, 32); +} diff --git a/yarn.lock b/yarn.lock index fc85386..f7c9802 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1978,7 +1978,7 @@ chai-as-promised@^7.1.1: dependencies: check-error "^1.0.2" -chai@^4.3.7: +chai@^4.2.0: version "4.4.0" resolved "https://registry.yarnpkg.com/chai/-/chai-4.4.0.tgz#f9ac79f26726a867ac9d90a9b382120479d5f55b" integrity sha512-x9cHNq1uvkCdU+5xTkNh5WtgD4e4yDFCsp9jVc7N7qVeKeftv3gO/ZrviX5d+3ZfxdYnZXZYujjRInu1RogU6A== @@ -5176,7 +5176,7 @@ solidity-ast@^0.4.51: dependencies: array.prototype.findlast "^1.2.2" -solidity-coverage@^0.8.0: +solidity-coverage@^0.8.5: version "0.8.5" resolved "https://registry.yarnpkg.com/solidity-coverage/-/solidity-coverage-0.8.5.tgz#64071c3a0c06a0cecf9a7776c35f49edc961e875" integrity sha512-6C6N6OV2O8FQA0FWA95FdzVH+L16HU94iFgg5wAFZ29UpLFkgNI/DRR2HotG1bC0F4gAc/OMs2BJI44Q/DYlKQ==