Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move beneficiary zero address check to Ownable #4531

Merged
merged 9 commits into from
Sep 4, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading