Move beneficiary zero address check to Ownable (#4531)

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com> Co-authored-by: Francisco <fg@frang.io>
OpenZeppelin · Sep 8, 2023 · 14d89c2 · 14d89c2
1 parent 9130516
commit 14d89c2
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 10 deletions.
diff --git a/.changeset/clever-bats-kick.md b/.changeset/clever-bats-kick.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': patch
+---
+
+`Ownable`: Prevent using address(0) as the initial owner.
diff --git a/contracts/access/Ownable.sol b/contracts/access/Ownable.sol
@@ -36,6 +36,9 @@ abstract contract Ownable is Context {
      * @dev Initializes the contract setting the address provided by the deployer as the initial owner.
      */
     constructor(address initialOwner) {
+        if (initialOwner == address(0)) {
+            revert OwnableInvalidOwner(address(0));
+        }
         _transferOwnership(initialOwner);
     }
 

diff --git a/contracts/finance/VestingWallet.sol b/contracts/finance/VestingWallet.sol
@@ -31,11 +31,6 @@ contract VestingWallet is Context, Ownable {
     event EtherReleased(uint256 amount);
     event ERC20Released(address indexed token, uint256 amount);
 
-    /**
-     * @dev The `beneficiary` is not a valid account.
-     */
-    error VestingWalletInvalidBeneficiary(address beneficiary);
-
     uint256 private _released;
     mapping(address token => uint256) private _erc20Released;
     uint64 private immutable _start;
@@ -46,10 +41,6 @@ contract VestingWallet is Context, Ownable {
      * vesting duration of the vesting wallet.
      */
     constructor(address beneficiary, uint64 startTimestamp, uint64 durationSeconds) payable Ownable(beneficiary) {
-        if (beneficiary == address(0)) {
-            revert VestingWalletInvalidBeneficiary(address(0));
-        }
-
         _start = startTimestamp;
         _duration = durationSeconds;
     }

diff --git a/test/access/Ownable.test.js b/test/access/Ownable.test.js
@@ -14,6 +14,10 @@ contract('Ownable', function (accounts) {
     this.ownable = await Ownable.new(owner);
   });
 
+  it('rejects zero address for initialOwner', async function () {
+    await expectRevertCustomError(Ownable.new(constants.ZERO_ADDRESS), 'OwnableInvalidOwner', [constants.ZERO_ADDRESS]);
+  });
+
   it('has an owner', async function () {
     expect(await this.ownable.owner()).to.equal(owner);
   });

diff --git a/test/finance/VestingWallet.test.js b/test/finance/VestingWallet.test.js
@@ -23,7 +23,7 @@ contract('VestingWallet', function (accounts) {
   it('rejects zero address for beneficiary', async function () {
     await expectRevertCustomError(
       VestingWallet.new(constants.ZERO_ADDRESS, this.start, duration),
-      'VestingWalletInvalidBeneficiary',
+      'OwnableInvalidOwner',
       [constants.ZERO_ADDRESS],
     );
   });