Skip to content

Commit

Permalink
feat: count number of sysAdmins, test: fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@kevin-fruitful
kevin-fruitful committed Mar 15, 2024
1 parent 04a2326 commit f7bfe2f
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 54 deletions.
4 changes: 4 additions & 0 deletions src/facets/ACLFacet.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,8 @@ contract ACLFacet is Modifiers {
function setMinter(address _newMinter) external onlySysAdmin {
LibACL._setMinter(_newMinter);
}

function removeSysAdmin(address _removeSystemAdmin) external onlySysAdmin {
LibACL._removeSystemAdmin(_removeSystemAdmin);
}
}
4 changes: 2 additions & 2 deletions src/facets/NaymsOwnershipFacet.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import { IERC173 } from "lib/diamond-2-hardhat/contracts/interfaces/IERC173.sol"
import { Modifiers } from "src/shared/Modifiers.sol";

contract NaymsOwnershipFacet is IERC173, Modifiers {
error NewSystemAdminCannotAlsoBeTheOwner();
error NewOwnerCannotAlsoBeSystemAdmin();

function transferOwnership(address _newOwner) external override onlySysAdmin {
AppStorage storage s = LibAppStorage.diamondStorage();
if (s.sysAdmins[_newOwner] == true) {
revert NewSystemAdminCannotAlsoBeTheOwner();
revert NewOwnerCannotAlsoBeSystemAdmin();
}

LibDiamond.setContractOwner(_newOwner);
Expand Down
14 changes: 14 additions & 0 deletions src/libs/LibACL.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,25 @@ import { AppStorage, LibAppStorage } from "../shared/AppStorage.sol";
library LibACL {
/// @dev The Nayms Diamond (proxy contract) owner (address) must be mutually exclusive with the system admin role.
error OwnerCannotBeSystemAdmin();
error MustHaveAtLeastOneSystemAdmin();

function _setSystemAdmin(address _newSystemAdmin) internal {
AppStorage storage s = LibAppStorage.diamondStorage();

s.sysAdmins[_newSystemAdmin] = true;
s.sysAdminsCount += 1;
}

function _removeSystemAdmin(address _systemAdmin) internal {
AppStorage storage s = LibAppStorage.diamondStorage();

if (s.sysAdmins[_systemAdmin]) {
if (s.sysAdminsCount == 1) {
revert MustHaveAtLeastOneSystemAdmin();
}
s.sysAdmins[_systemAdmin] = false;
s.sysAdminsCount -= 1;
}
}

function _setMinter(address _newMinter) internal {
Expand Down
1 change: 1 addition & 0 deletions src/libs/LibInitDiamond.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ library LibInitDiamond {
AppStorage storage s = LibAppStorage.diamondStorage();

s.sysAdmins[_newSystemAdmin] = true;
s.sysAdminsCount += 1;
}

function setMinter(address _newMinter) internal {
Expand Down
1 change: 1 addition & 0 deletions src/shared/AppStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ struct AppStorage {
uint256 upgradeExpiration; // the period of time that an upgrade is valid until.
//// ROLES, ACL ////
mapping(address sysAdmin => bool isSysAdmin) sysAdmins;
uint256 sysAdminsCount;
address minter;
}

Expand Down
115 changes: 63 additions & 52 deletions test/NaymToken.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ pragma solidity ^0.8.20;
import { Test, console as c, Vm } from "forge-std/Test.sol";

import { IERC20Errors } from "openzeppelin/interfaces/draft-IERC6093.sol";
import { Ownable } from "openzeppelin/access/Ownable.sol";
import { NaymsOwnershipFacet } from "src/facets/NaymsOwnershipFacet.sol";

import { IDiamondCut } from "lib/diamond-2-hardhat/contracts/interfaces/IDiamondCut.sol";
import { DiamondProxy } from "src/generated/DiamondProxy.sol";
import { IDiamondProxy } from "src/generated/IDiamondProxy.sol";
import { LibDiamondHelper } from "src/generated/LibDiamondHelper.sol";
import { LibGovernance } from "src/libs/LibGovernance.sol";
import { LibACL } from "src/libs/LibACL.sol";
import { LibHelpers } from "src/libs/LibHelpers.sol";
import { InitDiamond } from "src/init/InitDiamond.sol";
import { NaymsTokenFacet } from "src/facets/NaymsTokenFacet.sol";
Expand All @@ -27,7 +28,7 @@ contract NaymTokenTest is Test {
address minter2 = address(0x456);
address user1 = address(0x1234);

address public naymsAddress;
address public tAddress;

IDiamondProxy public t;
InitDiamond public initDiamond;
Expand All @@ -41,44 +42,46 @@ contract NaymTokenTest is Test {
c.log("block.chainid", block.chainid);

bool BOOL_FORK_TEST = vm.envOr({ name: "BOOL_FORK_TEST", defaultValue: false });
tAddress = vm.envOr({ name: "DIAMOND_ADDRESS", defaultValue: address(0) });

c.log("Are tests being run on a fork?".yellow().bold(), BOOL_FORK_TEST);
bool TESTS_FORK_UPGRADE_DIAMOND = vm.envOr({ name: "TESTS_FORK_UPGRADE_DIAMOND", defaultValue: true });
c.log("Are we testing diamond upgrades on a fork?".yellow().bold(), TESTS_FORK_UPGRADE_DIAMOND);

if (BOOL_FORK_TEST) {
// uint256 FORK_BLOCK = vm.envOr({
// name: string.concat("FORK_BLOCK_", vm.toString(block.chainid)),
// defaultValue: type(uint256).max
// });
// c.log("FORK_BLOCK", FORK_BLOCK);

// if (FORK_BLOCK == type(uint256).max) {
// c.log(
// "Using latest block for fork, consider pinning a block number to avoid overloading the RPC
// endpoint"
// );
// vm.createSelectFork(getChain(block.chainid).rpcUrl);
// } else {
// vm.createSelectFork(getChain(block.chainid).rpcUrl, FORK_BLOCK);
// }

// naymsAddress = getDiamondAddress();
// t = IDiamondProxy(naymsAddress);

// deployer = address(this);
// owner = t.owner();
// vm.label(owner, "Owner");
// systemAdmin = vm.envOr({
// name: string.concat("SYSTEM_ADMIN_", vm.toString(block.chainid)),
// defaultValue: address(0xE6aD24478bf7E1C0db07f7063A4019C83b1e5929)
// });
// vm.label(systemAdmin, "System Admin");

// vm.startPrank(owner);
// if (TESTS_FORK_UPGRADE_DIAMOND) {
// IDiamondCut.FacetCut[] memory cut = LibDiamondHelper.deployFacetsAndGetCuts(naymsAddress);
// scheduleAndUpgradeDiamond(cut);
// }
uint256 FORK_BLOCK = vm.envOr({
name: string.concat("FORK_BLOCK_", vm.toString(block.chainid)),
defaultValue: type(uint256).max
});
c.log("FORK_BLOCK", FORK_BLOCK);

if (FORK_BLOCK == type(uint256).max) {
c.log(
"Using latest block for fork, consider pinning a block number to avoid overloading the RPC endpoint"
);
vm.createSelectFork(getChain(block.chainid).rpcUrl);
} else {
vm.createSelectFork(getChain(block.chainid).rpcUrl, FORK_BLOCK);
}
// Get diamond address from env
tAddress = vm.envOr({ name: "DIAMOND_ADDRESS", defaultValue: address(0) });

t = IDiamondProxy(tAddress);

deployer = address(this);
owner = t.owner();
vm.label(owner, "Owner");
systemAdmin = vm.envOr({
name: string.concat("SYSTEM_ADMIN_", vm.toString(block.chainid)),
defaultValue: address(0xE6aD24478bf7E1C0db07f7063A4019C83b1e5929)
});
vm.label(systemAdmin, "System Admin");

vm.startPrank(owner);
if (TESTS_FORK_UPGRADE_DIAMOND) {
IDiamondCut.FacetCut[] memory cut = LibDiamondHelper.deployFacetsAndGetCuts(tAddress);
scheduleAndUpgradeDiamond(cut);
}
} else {
c.log("Local testing (no fork)");

Expand All @@ -90,9 +93,9 @@ contract NaymTokenTest is Test {
systemAdmin = makeAddr("System Admin 0");

c.log("Deploy diamond");
naymsAddress = address(new DiamondProxy(owner));
vm.label(naymsAddress, "Nayms diamond");
t = IDiamondProxy(naymsAddress);
tAddress = address(new DiamondProxy(owner));
vm.label(tAddress, "NAYM diamond");
t = IDiamondProxy(tAddress);

// deploy all facets
IDiamondCut.FacetCut[] memory cuts = LibDiamondHelper.deployFacetsAndGetCuts(address(t));
Expand All @@ -110,49 +113,51 @@ contract NaymTokenTest is Test {
vm.stopPrank();
}

function test_diamond() public {
t.name();
}

function test_Init() public {
assertEq(t.name(), "Naym");
assertEq(t.symbol(), "NAYM");
assertEq(t.decimals(), 18);
assertEq(t.totalSupply(), 0);
assertEq(t.owner(), owner1);
assertEq(t.minter(), minter1);
assertEq(t.owner(), address(this));
assertEq(t.minter(), systemAdmin);
}

function test_ChangeOwner() public {
assertEq(address(this), t.owner());

// pass
vm.prank(systemAdmin);
t.transferOwnership(owner2);
assertEq(t.owner(), owner2);

// not owner
vm.prank(systemAdmin);
vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, systemAdmin));
vm.expectRevert(abi.encodeWithSelector(NaymsOwnershipFacet.NewOwnerCannotAlsoBeSystemAdmin.selector));
t.transferOwnership(systemAdmin);
}

// invalid new owner
vm.prank(owner2);
vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableInvalidOwner.selector, address(0)));
t.transferOwnership(address(0));
function test_ChangeSystemAdmin() public {
vm.startPrank(systemAdmin);
vm.expectRevert(abi.encodeWithSelector(LibACL.MustHaveAtLeastOneSystemAdmin.selector));
t.removeSysAdmin(systemAdmin);

t.setSystemAdmin(owner1);
t.removeSysAdmin(systemAdmin);
}

function test_SetMinter() public {
// unauthorized
vm.prank(address(0x1234));
vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, address(0x1234)));
vm.expectRevert(abi.encodeWithSelector(Modifiers.NotSystemAdmin.selector, address(0x1234)));
t.setMinter(minter2);

// authorized
vm.prank(owner1);
vm.prank(systemAdmin);
t.setMinter(minter2);
assertEq(t.minter(), minter2);

// can set to null address
vm.prank(owner1);
vm.prank(systemAdmin);
t.setMinter(address(0));
assertEq(t.minter(), address(0));
}
Expand All @@ -163,6 +168,9 @@ contract NaymTokenTest is Test {
vm.expectRevert(abi.encodeWithSelector(Modifiers.UnauthorizedMinter.selector, user1));
t.mint(user1, 100);

vm.prank(systemAdmin);
t.setMinter(minter1);

// authorized
vm.prank(minter1);
t.mint(user1, 100);
Expand All @@ -171,6 +179,9 @@ contract NaymTokenTest is Test {
}

function test_Burn() public {
vm.prank(systemAdmin);
t.setMinter(minter1);

vm.prank(minter1);
t.mint(user1, 100);

Expand Down

0 comments on commit f7bfe2f

Please sign in to comment.