Skip to content

Commit

Permalink
Move beneficiary zero address check to Ownable (#4531)
Browse files Browse the repository at this point in the history
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Francisco <fg@frang.io>
  • Loading branch information
3 people authored Sep 4, 2023
1 parent 98b83df commit e7ba2f7
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changeset/clever-bats-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': patch
---

`Ownable`: Prevent using address(0) as the initial owner.
3 changes: 3 additions & 0 deletions contracts/access/Ownable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
9 changes: 0 additions & 9 deletions contracts/finance/VestingWallet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down
4 changes: 4 additions & 0 deletions test/access/Ownable.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
2 changes: 1 addition & 1 deletion test/finance/VestingWallet.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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],
);
});
Expand Down

0 comments on commit e7ba2f7

Please sign in to comment.