Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat fix selector role #13

Merged
merged 2 commits into from
Jan 16, 2024
Merged

Feat fix selector role #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9d5fc0
fix: selector role interface
seinmyung25 Jan 15, 2024
826c424
fix: selector interface test
seinmyung25 Jan 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions contracts/access/SelectorRoleControlUpgradeable.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ interface IMulticall {

// IEcoOwnable, TODO: check redundant override, already defined & implemented
interface ISelectorRoleControl is IAccessControlEnumerable, IPausable {
function grantSelectorRole(bytes4 selector, address account) external;

function revokeSelectorRole(bytes4 selector, address account) external;

function pause() external;

function unpause() external;
Expand All @@ -40,23 +44,17 @@ contract SelectorRoleControlUpgradeable is
_;
}

// owner or hash role
// owner or selector role
function _onlyAdmin(address account) internal view {
if (owner() != account) _checkRole(msg.sig, account);
}

function grantRole(
bytes32 role,
address account
) public virtual override(IAccessControl, AccessControlUpgradeable) onlyAdmin {
require(_grantRole(role, account), "role exist");
function grantSelectorRole(bytes4 selector, address account) public virtual override onlyAdmin {
require(_grantRole(selector, account), "role exist");
}

function revokeRole(
bytes32 role,
address account
) public virtual override(IAccessControl, AccessControlUpgradeable) onlyAdmin {
require(_revokeRole(role, account), "role not exist");
function revokeSelectorRole(bytes4 selector, address account) public virtual override onlyAdmin {
require(_revokeRole(selector, account), "role not exist");
}

function paused() public view virtual override returns (bool) {
Expand Down
28 changes: 14 additions & 14 deletions test/access/SelectorRoleControlUpgradeable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,36 +38,36 @@ describe("SelectorRoleControlUpgradeable", function () {
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.revokeSelectorRole(getSelector(role.pause), admin)).reverted;
await expect(role.grantSelectorRole(getSelector(role.pause), admin)).not.reverted;
await expect(role.grantSelectorRole(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.revokeSelectorRole(getSelector(role.unpause), admin)).reverted;
await expect(role.grantSelectorRole(getSelector(role.unpause), admin)).not.reverted;
await expect(role.grantSelectorRole(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.revokeSelectorRole(getSelector(role.pause), admin)).not.reverted;
await expect(role.revokeSelectorRole(getSelector(role.pause), admin)).reverted;

await expect(role.revokeRole(getSelector(role.unpause), admin)).not.reverted;
await expect(role.revokeRole(getSelector(role.unpause), admin)).reverted;
await expect(role.revokeSelectorRole(getSelector(role.unpause), admin)).not.reverted;
await expect(role.revokeSelectorRole(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).grantSelectorRole(getSelector(role.pause), admin)).reverted;
await expect(role.connect(admin).grantSelectorRole(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;
await expect(role.connect(admin).revokeSelectorRole(getSelector(role.pause), admin)).reverted;
await expect(role.connect(admin).revokeSelectorRole(getSelector(role.unpause), admin)).reverted;
});
});
});
2 changes: 1 addition & 1 deletion test/helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import hre from "hardhat";

export function getSelector(contractMethod: { fragment: { selector: string } }): string {
return hre.ethers.zeroPadBytes(contractMethod.fragment.selector, 32);
return contractMethod.fragment.selector;
}

export interface HardhatContract {
Expand Down
7 changes: 4 additions & 3 deletions test/proxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import hre from "hardhat";

import { EcoERC20Mintable } from "../typechain-types";

import { getSelector } from "./helper";

type ProxiedContract<FT extends ContractFactory> = Awaited<ReturnType<FT["deploy"]>>;

class ProxyContractTypeTest<FT extends ContractFactory> extends AsyncConstructor {
Expand Down Expand Up @@ -100,13 +102,12 @@ describe("ERC20 Mintable", function () {
it("Shouldn't fail mint with the right role access account", async function () {
const { pErc20, users } = await loadFixture(NFT_Mintable_Fixture);

const nextMintSelector = hre.ethers.zeroPadBytes(pErc20.mint.fragment.selector, 32);
await expect(pErc20.grantRole(nextMintSelector, users[0])).not.reverted;
await expect(pErc20.grantSelectorRole(getSelector(pErc20.mint), users[0])).not.reverted;

const user_connected_nft = pErc20.connect(users[0]);
await expect(user_connected_nft.mint(users[0], amount)).not.reverted;

await expect(pErc20.revokeRole(nextMintSelector, users[0])).not.reverted;
await expect(pErc20.revokeSelectorRole(getSelector(pErc20.mint), users[0])).not.reverted;
await expect(user_connected_nft.mint(users[0], amount)).reverted;
});
});
Expand Down
7 changes: 4 additions & 3 deletions test/token/ERC20.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { loadFixture } from "@nomicfoundation/hardhat-toolbox/network-helpers";
import { expect } from "chai";
import hre from "hardhat";

import { getSelector } from "../helper";

describe("ERC20 Mintable", function () {
const name = "Mintable Token";
const symbol = "M ERC20";
Expand Down Expand Up @@ -52,13 +54,12 @@ describe("ERC20 Mintable", function () {
it("Shouldn't fail mint with the right role access account", async function () {
const { erc20, users } = await loadFixture(NFT_Mintable_Fixture);

const nextMintSelector = hre.ethers.zeroPadBytes(erc20.mint.fragment.selector, 32);
await expect(erc20.grantRole(nextMintSelector, users[0])).not.reverted;
await expect(erc20.grantSelectorRole(getSelector(erc20.mint), users[0])).not.reverted;

const user_connected_nft = erc20.connect(users[0]);
await expect(user_connected_nft.mint(users[0], amount)).not.reverted;

await expect(erc20.revokeRole(nextMintSelector, users[0])).not.reverted;
await expect(erc20.revokeSelectorRole(getSelector(erc20.mint), users[0])).not.reverted;
await expect(user_connected_nft.mint(users[0], amount)).reverted;
});
});
Expand Down
4 changes: 2 additions & 2 deletions test/token/NFT/NFT_Mintable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ describe("NFT Mintable", function () {

await expect(nft.connect(admin).nextMint(user0)).reverted;

await expect(nft.grantRole(getSelector(nft.nextMint), admin)).not.reverted;
await expect(nft.grantSelectorRole(getSelector(nft.nextMint), admin)).not.reverted;
const tokenId = await nft.nextMintId();
await expect(nft.connect(admin).nextMint(user0))
.emit(nft, "Transfer")
.withArgs(hre.ethers.ZeroAddress, user0, tokenId);
await expect(nft.connect(user0).nextMint(user0)).reverted;
await expect(nft.revokeRole(getSelector(nft.nextMint), admin)).not.reverted;
await expect(nft.revokeSelectorRole(getSelector(nft.nextMint), admin)).not.reverted;
});

describe("Transfer", function () {
Expand Down