diff --git a/contracts/common/Initializable.sol b/contracts/common/Initializable.sol index af84fbe..905d424 100644 --- a/contracts/common/Initializable.sol +++ b/contracts/common/Initializable.sol @@ -24,11 +24,18 @@ contract Initializable { */ bool private initializing; + /** + * @dev The contract instance has already been initialized. + */ + error ContractInitialized(); + /** * @dev Modifier to use in the initializer function of a contract. */ modifier initializer() { - require(initializing || !initialized, "Contract instance has already been initialized"); + if (!initializing && initialized) { + revert ContractInitialized(); + } bool isTopLevelCall = !initializing; if (isTopLevelCall) { diff --git a/contracts/common/ReentrancyGuard.sol b/contracts/common/ReentrancyGuard.sol index 8d1d296..3c7f1d2 100644 --- a/contracts/common/ReentrancyGuard.sol +++ b/contracts/common/ReentrancyGuard.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "./Initializable.sol"; +import {Initializable} from "./Initializable.sol"; /** * @dev Contract module that helps prevent reentrant calls to a function. @@ -19,6 +19,11 @@ contract ReentrancyGuard is Initializable { // counter to allow mutex lock with only one SSTORE operation uint256 private _guardCounter; + /** + * @dev Reentrant call. + */ + error ReentrantCall(); + function initialize() internal initializer { // The counter starts at one to prevent changing it from zero to a non-zero // value, which is a more expensive operation. @@ -36,7 +41,9 @@ contract ReentrancyGuard is Initializable { _guardCounter += 1; uint256 localCounter = _guardCounter; _; - require(localCounter == _guardCounter, "ReentrancyGuard: reentrant call"); + if (localCounter != _guardCounter) { + revert ReentrantCall(); + } } uint256[50] private ______gap; diff --git a/contracts/ownership/Ownable.sol b/contracts/ownership/Ownable.sol index 3466077..85780ef 100644 --- a/contracts/ownership/Ownable.sol +++ b/contracts/ownership/Ownable.sol @@ -1,8 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "../common/Initializable.sol"; - +import {Initializable} from "../common/Initializable.sol"; /** * @dev Contract module which provides a basic access control mechanism, where * there is an account (an owner) that can be granted exclusive access to @@ -15,6 +14,16 @@ import "../common/Initializable.sol"; contract Ownable is Initializable { address private _owner; + /** + * @dev The caller account is not authorized to perform an operation. + */ + error OwnableUnauthorizedAccount(address account); + + /** + * @dev The owner is not a valid owner account. (eg. `address(0)`) + */ + error OwnableInvalidOwner(address owner); + event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); /** @@ -36,7 +45,9 @@ contract Ownable is Initializable { * @dev Throws if called by any account other than the owner. */ modifier onlyOwner() { - require(isOwner(), "Ownable: caller is not the owner"); + if (!isOwner()) { + revert OwnableUnauthorizedAccount(msg.sender); + } _; } @@ -71,7 +82,9 @@ contract Ownable is Initializable { * @dev Transfers ownership of the contract to a new account (`newOwner`). */ function _transferOwnership(address newOwner) internal { - require(newOwner != address(0), "Ownable: new owner is the zero address"); + if (newOwner == address(0)) { + revert OwnableInvalidOwner(address(0)); + } emit OwnershipTransferred(_owner, newOwner); _owner = newOwner; } diff --git a/test/NodeDriver.ts b/test/NodeDriver.ts index 87bb017..937d2bd 100644 --- a/test/NodeDriver.ts +++ b/test/NodeDriver.ts @@ -38,8 +38,9 @@ describe('NodeDriver', () => { it('Should revert when not owner', async function () { const account = ethers.Wallet.createRandom(); - await expect(this.nodeDriverAuth.connect(this.nonOwner).migrateTo(account)).to.be.revertedWith( - 'Ownable: caller is not the owner', + await expect(this.nodeDriverAuth.connect(this.nonOwner).migrateTo(account)).to.be.revertedWithCustomError( + this.nodeDriverAuth, + 'OwnableUnauthorizedAccount', ); }); }); @@ -52,9 +53,9 @@ describe('NodeDriver', () => { it('Should revert when not owner', async function () { const address = ethers.Wallet.createRandom(); - await expect(this.nodeDriverAuth.connect(this.nonOwner).copyCode(this.sfc, address)).to.be.revertedWith( - 'Ownable: caller is not the owner', - ); + await expect( + this.nodeDriverAuth.connect(this.nonOwner).copyCode(this.sfc, address), + ).to.be.revertedWithCustomError(this.nodeDriverAuth, 'OwnableUnauthorizedAccount'); }); }); @@ -66,8 +67,9 @@ describe('NodeDriver', () => { }); it('Should revert when not owner', async function () { - await expect(this.nodeDriverAuth.connect(this.nonOwner).updateNetworkVersion(1)).to.be.revertedWith( - 'Ownable: caller is not the owner', + await expect(this.nodeDriverAuth.connect(this.nonOwner).updateNetworkVersion(1)).to.be.revertedWithCustomError( + this.nodeDriverAuth, + 'OwnableUnauthorizedAccount', ); }); }); @@ -78,8 +80,9 @@ describe('NodeDriver', () => { }); it('Should revert when not owner', async function () { - await expect(this.nodeDriverAuth.connect(this.nonOwner).advanceEpochs(10)).to.be.revertedWith( - 'Ownable: caller is not the owner', + await expect(this.nodeDriverAuth.connect(this.nonOwner).advanceEpochs(10)).to.be.revertedWithCustomError( + this.nodeDriverAuth, + 'OwnableUnauthorizedAccount', ); }); }); diff --git a/test/SFC.ts b/test/SFC.ts index f00b9ba..d21cc0a 100644 --- a/test/SFC.ts +++ b/test/SFC.ts @@ -290,15 +290,16 @@ describe('SFC', () => { }); it('Should revert when transferring ownership if not owner', async function () { - await expect(this.sfc.connect(this.user).transferOwnership(ethers.ZeroAddress)).to.be.revertedWith( - 'Ownable: caller is not the owner', + await expect(this.sfc.connect(this.user).transferOwnership(ethers.ZeroAddress)).to.be.revertedWithCustomError( + this.nodeDriverAuth, + 'OwnableUnauthorizedAccount', ); }); it('Should revert when transferring ownership to zero address', async function () { - await expect(this.sfc.transferOwnership(ethers.ZeroAddress)).to.be.revertedWith( - 'Ownable: new owner is the zero address', - ); + await expect(this.sfc.transferOwnership(ethers.ZeroAddress)) + .to.be.revertedWithCustomError(this.nodeDriverAuth, 'OwnableInvalidOwner') + .withArgs(ethers.ZeroAddress); }); });