Skip to content

Commit

Permalink
refactor: use oz ownable opposed to custom copy (#60)
Browse files Browse the repository at this point in the history
  • Loading branch information
sakulstra authored Jan 27, 2025
1 parent 804c23f commit c686d34
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 118 deletions.
9 changes: 5 additions & 4 deletions src/contracts/access-control/OwnableWithGuardian.sol
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.7.0;

import {Ownable} from 'openzeppelin-contracts/contracts/access/Ownable.sol';
import {IWithGuardian} from './interfaces/IWithGuardian.sol';
import {Ownable} from '../oz-common/Ownable.sol';

abstract contract OwnableWithGuardian is Ownable, IWithGuardian {
address private _guardian;

constructor() {
constructor(address initialOwner) Ownable(initialOwner) {
_updateGuardian(_msgSender());
}

Expand Down Expand Up @@ -41,10 +41,11 @@ abstract contract OwnableWithGuardian is Ownable, IWithGuardian {
}

function _checkGuardian() internal view {
require(guardian() == _msgSender(), 'ONLY_BY_GUARDIAN');
if (guardian() != _msgSender()) revert OnlyGuardianInvalidCaller(_msgSender());
}

function _checkOwnerOrGuardian() internal view {
require(_msgSender() == owner() || _msgSender() == guardian(), 'ONLY_BY_OWNER_OR_GUARDIAN');
if (_msgSender() != owner() && _msgSender() != guardian())
revert OnlyGuardianOrOwnerInvalidCaller(_msgSender());
}
}
10 changes: 0 additions & 10 deletions src/contracts/access-control/UpgradeableOwnableWithGuardian.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,6 @@ abstract contract UpgradeableOwnableWithGuardian is OwnableUpgradeable, IWithGua
}
}

/**
* @dev The caller account is not authorized to perform an operation.
*/
error OnlyGuardianInvalidCaller(address account);

/**
* @dev The caller account is not authorized to perform an operation.
*/
error OnlyGuardianOrOwnerInvalidCaller(address account);

/**
* @dev Initializes the contract setting the address provided by the deployer as the initial owner.
*/
Expand Down
10 changes: 10 additions & 0 deletions src/contracts/access-control/interfaces/IWithGuardian.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@ interface IWithGuardian {
*/
event GuardianUpdated(address oldGuardian, address newGuardian);

/**
* @dev The caller account is not authorized to perform an operation.
*/
error OnlyGuardianInvalidCaller(address account);

/**
* @dev The caller account is not authorized to perform an operation.
*/
error OnlyGuardianOrOwnerInvalidCaller(address account);

/**
* @dev get guardian address;
*/
Expand Down
84 changes: 0 additions & 84 deletions src/contracts/oz-common/Ownable.sol

This file was deleted.

14 changes: 10 additions & 4 deletions test/OwnableWithGuardian.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
pragma solidity ^0.8.0;

import 'forge-std/Test.sol';
import {OwnableWithGuardian} from '../src/contracts/access-control/OwnableWithGuardian.sol';
import {OwnableWithGuardian, IWithGuardian} from '../src/contracts/access-control/OwnableWithGuardian.sol';

contract ImplOwnableWithGuardian is OwnableWithGuardian {
constructor(address initialOwner) OwnableWithGuardian(initialOwner) {}

function mock_onlyGuardian() external onlyGuardian {}

function mock_onlyOwnerOrGuardian() external onlyOwnerOrGuardian {}
Expand All @@ -17,7 +19,7 @@ contract TestOfOwnableWithGuardian is Test {
address guardian = makeAddr('guardian');

function setUp() public {
withGuardian = new ImplOwnableWithGuardian();
withGuardian = new ImplOwnableWithGuardian(address(this));
assertEq(withGuardian.owner(), address(this));
assertEq(withGuardian.guardian(), address(this));
withGuardian.transferOwnership(owner);
Expand All @@ -40,7 +42,9 @@ contract TestOfOwnableWithGuardian is Test {
}

function testGuardianUpdateNoAccess() external {
vm.expectRevert('ONLY_BY_OWNER_OR_GUARDIAN');
vm.expectRevert(
abi.encodeWithSelector(IWithGuardian.OnlyGuardianOrOwnerInvalidCaller.selector, address(this))
);
withGuardian.updateGuardian(guardian);
}

Expand All @@ -50,7 +54,9 @@ contract TestOfOwnableWithGuardian is Test {
}

function test_onlyGuardianGuard_shouldRevert() external {
vm.expectRevert('ONLY_BY_GUARDIAN');
vm.expectRevert(
abi.encodeWithSelector(IWithGuardian.OnlyGuardianInvalidCaller.selector, address(this))
);
withGuardian.mock_onlyGuardian();
}
}
17 changes: 4 additions & 13 deletions test/UpgradeableOwnableWithGuardian.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity ^0.8.20;

import 'forge-std/Test.sol';
import {UpgradeableOwnableWithGuardian} from '../src/contracts/access-control/UpgradeableOwnableWithGuardian.sol';
import {UpgradeableOwnableWithGuardian, IWithGuardian} from '../src/contracts/access-control/UpgradeableOwnableWithGuardian.sol';

contract ImplOwnableWithGuardian is UpgradeableOwnableWithGuardian {
function initialize(address owner, address guardian) public initializer {
Expand Down Expand Up @@ -33,20 +33,14 @@ contract TestOfUpgradableOwnableWithGuardian is Test {

function test_onlyGuardian() external {
vm.expectRevert(
abi.encodeWithSelector(
UpgradeableOwnableWithGuardian.OnlyGuardianInvalidCaller.selector,
address(this)
)
abi.encodeWithSelector(IWithGuardian.OnlyGuardianInvalidCaller.selector, address(this))
);
ImplOwnableWithGuardian(address(withGuardian)).mock_onlyGuardian();
}

function test_onlyOwnerOrGuardian() external {
vm.expectRevert(
abi.encodeWithSelector(
UpgradeableOwnableWithGuardian.OnlyGuardianOrOwnerInvalidCaller.selector,
address(this)
)
abi.encodeWithSelector(IWithGuardian.OnlyGuardianOrOwnerInvalidCaller.selector, address(this))
);
ImplOwnableWithGuardian(address(withGuardian)).mock_onlyOwnerOrGuardian();
}
Expand All @@ -66,10 +60,7 @@ contract TestOfUpgradableOwnableWithGuardian is Test {

vm.prank(eoa);
vm.expectRevert(
abi.encodeWithSelector(
UpgradeableOwnableWithGuardian.OnlyGuardianOrOwnerInvalidCaller.selector,
eoa
)
abi.encodeWithSelector(IWithGuardian.OnlyGuardianOrOwnerInvalidCaller.selector, eoa)
);
withGuardian.updateGuardian(newGuardian);
}
Expand Down
5 changes: 2 additions & 3 deletions test/create3Test.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,15 @@
pragma solidity ^0.8.0;

import 'forge-std/Test.sol';
import {Ownable} from 'openzeppelin-contracts/contracts/access/Ownable.sol';
import {Create3Factory, ICreate3Factory, Create3} from '../src/contracts/create3/Create3Factory.sol';
import {Ownable} from '../src/contracts/oz-common/Ownable.sol';

contract MockContract is Ownable {
address public immutable SOME_ADDRESS;
address internal _someOtherAddress;

constructor(address someAddress, address owner) {
constructor(address someAddress, address owner) Ownable(owner) {
SOME_ADDRESS = someAddress;
_transferOwnership(owner);
}

function getOtherAddress() external view returns (address) {
Expand Down

0 comments on commit c686d34

Please sign in to comment.