Skip to content

Commit

Permalink
Add custom error types
Browse files Browse the repository at this point in the history
  • Loading branch information
@ermyas
ermyas committed May 14, 2024
1 parent 219823c commit 81df8e2
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 44 deletions.
93 changes: 59 additions & 34 deletions contracts/deployer/AccessControlledDeployer.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,29 @@ contract AccessControlledDeployer is AccessControlEnumerable, Pausable {
/// @notice Role identifier for those who can deploy contracts
bytes32 public constant DEPLOYER_ROLE = keccak256("DEPLOYER");

/// @notice Emitted when the zero address is provided when it is not expected
error ZeroAddress();

/// @notice Emitted when a provided list of deployer addresses is empty
error EmptyDeployerList();

/// @notice Emitted if the caller, this contract, is not the owner of the targeted deployer
error NotOwnerOfDeployer();

/**
* @notice Construct a new RBACDeployer contract
* @param admin The address to grant the DEFAULT_ADMIN_ROLE
* @param pauser The address to grant the PAUSER_ROLE
* @param unpauser The address to grant the UNPAUSER_ROLE
*/
constructor(address admin, address pauser, address unpauser) {
require(admin != address(0), "admin is the zero address");
require(pauser != address(0), "pauser is the zero address");
require(unpauser != address(0), "unpauser is the zero address");
if (admin == address(0) || pauser == address(0) || unpauser == address(0)) {
revert ZeroAddress();
}

_setupRole(DEFAULT_ADMIN_ROLE, admin);
_setupRole(PAUSER_ROLE, pauser);
_setupRole(UNPAUSER_ROLE, unpauser);
_grantRole(DEFAULT_ADMIN_ROLE, admin);
_grantRole(PAUSER_ROLE, pauser);
_grantRole(UNPAUSER_ROLE, unpauser);
}

/**
Expand All @@ -39,18 +48,20 @@ contract AccessControlledDeployer is AccessControlEnumerable, Pausable {
* @param bytecode The bytecode of the contract to be deployed
* @param salt A salt to influence the contract address
* @dev Only address with DEPLOYER_ROLE can call this function
* @dev This function requires that the current owner of `deployer` is this contract
* @dev The function can only be called if the contract is not in a paused state
* @dev The function emits `Deployed` event after the contract is deployed
* @return The address of the deployed contract
*/
function deploy(IDeployer deployer, bytes memory bytecode, bytes32 salt)
external
payable
whenNotPaused
onlyRole(DEPLOYER_ROLE)
returns (address)
{
require(address(deployer) != address(0), "deployer contract is the zero address");
return deployer.deploy(bytecode, salt);
function deploy(
IDeployer deployer,
bytes memory bytecode,
bytes32 salt
) external payable whenNotPaused onlyRole(DEPLOYER_ROLE) returns (address) {
if (address(deployer) == address(0)) {
revert ZeroAddress();
}
return deployer.deploy{value: msg.value}(bytecode, salt);
}

/**
Expand All @@ -60,17 +71,20 @@ contract AccessControlledDeployer is AccessControlEnumerable, Pausable {
* @param salt A salt to influence the contract address
* @param init Init data used to initialize the deployed contract
* @dev Only address with DEPLOYER_ROLE can call this function
* @dev This function requires that the current owner of `deployer` is this contract
* @dev The function can only be called if the contract is not in a paused state
* @dev The function emits `Deployed` event after the contract is deployed
* @return The address of the deployed contract
*/
function deployAndInit(IDeployer deployer, bytes memory bytecode, bytes32 salt, bytes calldata init)
external
payable
whenNotPaused
onlyRole(DEPLOYER_ROLE)
returns (address)
{
require(address(deployer) != address(0), "deployer contract is the zero address");
function deployAndInit(
IDeployer deployer,
bytes memory bytecode,
bytes32 salt,
bytes calldata init
) external payable whenNotPaused onlyRole(DEPLOYER_ROLE) returns (address) {
if (address(deployer) == address(0)) {
revert ZeroAddress();
}
return deployer.deployAndInit{value: msg.value}(bytecode, salt, init);
}

Expand All @@ -82,9 +96,13 @@ contract AccessControlledDeployer is AccessControlEnumerable, Pausable {
* This is not emitted if an address is already a deployer
*/
function grantDeployerRole(address[] memory deployers) public {
require(deployers.length > 0, "deployers list is empty");
if (deployers.length == 0) {
revert EmptyDeployerList();
}
for (uint256 i = 0; i < deployers.length; i++) {
require(deployers[i] != address(0), "deployer is the zero address");
if (deployers[i] == address(0)) {
revert ZeroAddress();
}
grantRole(DEPLOYER_ROLE, deployers[i]);
}
}
Expand All @@ -97,9 +115,13 @@ contract AccessControlledDeployer is AccessControlEnumerable, Pausable {
* This is not emitted if an address was not a deployer
*/
function revokeDeployerRole(address[] memory deployers) public {
require(deployers.length > 0, "deployers list is empty");
if (deployers.length == 0) {
revert EmptyDeployerList();
}
for (uint256 i = 0; i < deployers.length; i++) {
require(deployers[i] != address(0), "deployer is the zero address");
if (deployers[i] == address(0)) {
revert ZeroAddress();
}
revokeRole(DEPLOYER_ROLE, deployers[i]);
}
}
Expand All @@ -111,13 +133,16 @@ contract AccessControlledDeployer is AccessControlEnumerable, Pausable {
* @dev Only address with DEFAULT_ADMIN_ROLE can call this function
* @dev This function requires that the current owner of `ownableDeployer` is this contract
*/
function transferOwnershipOfDeployer(Ownable ownableDeployer, address newOwner)
external
onlyRole(DEFAULT_ADMIN_ROLE)
{
require(address(ownableDeployer) != address(0), "deployer contract is the zero address");
require(newOwner != address(0), "new owner is the zero address");
require(ownableDeployer.owner() == address(this), "deployer contract is not owned by this contract");
function transferOwnershipOfDeployer(
Ownable ownableDeployer,
address newOwner
) external onlyRole(DEFAULT_ADMIN_ROLE) {
if (address(ownableDeployer) == address(0) || newOwner == address(0)) {
revert ZeroAddress();
}
if (ownableDeployer.owner() != address(this)) {
revert NotOwnerOfDeployer();
}
ownableDeployer.transferOwnership(newOwner);
}

Expand Down
24 changes: 14 additions & 10 deletions test/deployer/AccessControlledDeployer.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ contract AccessControlledDeployerTest is Test, Create2Utils, Create3Utils {

event Deployed(address indexed deployedAddress, address indexed sender, bytes32 indexed salt, bytes32 bytecodeHash);

error ZeroAddress();
error EmptyDeployerList();
error NotOwnerOfDeployer();

function setUp() public {
rbacDeployer = new AccessControlledDeployer(admin, pauser, unpauser);

Expand All @@ -38,17 +42,17 @@ contract AccessControlledDeployerTest is Test, Create2Utils, Create3Utils {
* Constructor
*/
function test_Constructor_RevertIf_AdminIsZeroAddress() public {
vm.expectRevert("admin is the zero address");
vm.expectRevert(ZeroAddress.selector);
new AccessControlledDeployer(address(0), pauser, unpauser);
}

function test_Constructor_RevertIf_PauserIsZeroAddress() public {
vm.expectRevert("pauser is the zero address");
vm.expectRevert(ZeroAddress.selector);
new AccessControlledDeployer(admin, address(0), unpauser);
}

function test_Constructor_RevertIf_UnpauserIsZeroAddress() public {
vm.expectRevert("unpauser is the zero address");
vm.expectRevert(ZeroAddress.selector);
new AccessControlledDeployer(admin, pauser, address(0));
}

Expand Down Expand Up @@ -91,20 +95,20 @@ contract AccessControlledDeployerTest is Test, Create2Utils, Create3Utils {
function test_RevertIf_TransferDeployerOwnership_WithZeroOwnerAddress() public {
OwnableCreate2Deployer create2Deployer = new OwnableCreate2Deployer(address(rbacDeployer));
vm.startPrank(admin);
vm.expectRevert("new owner is the zero address");
vm.expectRevert(ZeroAddress.selector);
rbacDeployer.transferOwnershipOfDeployer(create2Deployer, address(0));
}

function test_RevertIf_TransferDeployerOwnership_WhenNotCurrentOwner() public {
OwnableCreate2Deployer create2Deployer = new OwnableCreate2Deployer(makeAddr("currentOwner"));
vm.startPrank(admin);
vm.expectRevert("deployer contract is not owned by this contract");
vm.expectRevert(NotOwnerOfDeployer.selector);
rbacDeployer.transferOwnershipOfDeployer(create2Deployer, makeAddr("newOwner2"));
}

function test_RevertIf_TransferDeployerOwnership_WithZeroDeployerAddress() public {
vm.startPrank(admin);
vm.expectRevert("deployer contract is the zero address");
vm.expectRevert(ZeroAddress.selector);
rbacDeployer.transferOwnershipOfDeployer(Ownable(address(0)), makeAddr("newOwner2"));
}

Expand Down Expand Up @@ -169,7 +173,7 @@ contract AccessControlledDeployerTest is Test, Create2Utils, Create3Utils {
*/
function test_RevertIf_GrantDeployerRole_WithEmptyArray() public {
address[] memory emptyDeployers = new address[](0);
vm.expectRevert("deployers list is empty");
vm.expectRevert(EmptyDeployerList.selector);
vm.prank(admin);
rbacDeployer.grantDeployerRole(emptyDeployers);
}
Expand All @@ -180,7 +184,7 @@ contract AccessControlledDeployerTest is Test, Create2Utils, Create3Utils {
// note that second deployer in the array is the zero address

vm.prank(admin);
vm.expectRevert("deployer is the zero address");
vm.expectRevert(ZeroAddress.selector);
rbacDeployer.grantDeployerRole(newDeployers);
}

Expand Down Expand Up @@ -213,7 +217,7 @@ contract AccessControlledDeployerTest is Test, Create2Utils, Create3Utils {

function test_RevertIf_RevokeDeployerRole_WithEmptyArray() public {
address[] memory emptyDeployers = new address[](0);
vm.expectRevert("deployers list is empty");
vm.expectRevert(EmptyDeployerList.selector);
vm.prank(admin);
rbacDeployer.revokeDeployerRole(emptyDeployers);
}
Expand All @@ -224,7 +228,7 @@ contract AccessControlledDeployerTest is Test, Create2Utils, Create3Utils {
// note that second deployer in the array is the zero address

vm.prank(admin);
vm.expectRevert("deployer is the zero address");
vm.expectRevert(ZeroAddress.selector);
rbacDeployer.grantDeployerRole(existingDeployers);
}

Expand Down

0 comments on commit 81df8e2

Please sign in to comment.