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

Add Ownable2Step extension with 2-step transfer #3620

Merged
merged 38 commits into from
Sep 1, 2022
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
fccb61f
Change _owner to internal
heldersepu Aug 16, 2022
918ab11
Revert change to _owner variable
heldersepu Aug 16, 2022
00aae11
New contract SafeOwnable
heldersepu Aug 16, 2022
16f6014
SafeOwnableMock
heldersepu Aug 16, 2022
fbc33a8
testing the new SafeOwnable contract
heldersepu Aug 16, 2022
229b1d0
format code
heldersepu Aug 16, 2022
4b3369a
Add entry to changelog
heldersepu Aug 16, 2022
6c7325d
Fix error 'safe_ownable' is not in camel case
heldersepu Aug 16, 2022
80b5260
Fix mix of tabs and spaces
heldersepu Aug 16, 2022
db4af81
Allow address(0) as new owner
heldersepu Aug 16, 2022
c21d53d
Merge branch 'master' into master
heldersepu Aug 17, 2022
2530cd5
Rename _newOwner to _pendingOwner
heldersepu Aug 17, 2022
de9beed
Remove unused const
heldersepu Aug 17, 2022
c06b773
Remove unused constants
heldersepu Aug 17, 2022
3279163
override _transferOwnership to delete _pendingOwner
heldersepu Aug 17, 2022
d995d7c
Update contracts/access/SafeOwnable.sol
heldersepu Aug 17, 2022
b12b1e7
Update contracts/access/SafeOwnable.sol
heldersepu Aug 17, 2022
8d62986
format code
heldersepu Aug 17, 2022
320aa21
Add test to forceTransferOwnership
heldersepu Aug 17, 2022
fbf41df
Add check on pendingOwner
heldersepu Aug 17, 2022
20aa36c
Merge branch 'master' into master
heldersepu Aug 22, 2022
2ba7878
Merge branch 'master' into master
heldersepu Aug 24, 2022
9bbeb47
Rename to Ownable2Step
heldersepu Aug 24, 2022
16d5c16
Rename mock to Ownable2StepMock
heldersepu Aug 24, 2022
baf063e
Update CHANGELOG.md
heldersepu Aug 24, 2022
adcf9aa
Update contracts/access/Ownable2Step.sol
heldersepu Aug 24, 2022
0b46f49
check the event arguments
heldersepu Aug 24, 2022
50ca6bc
Merge branch 'master' of https://github.com/heldersepu/openzeppelin-c…
heldersepu Aug 24, 2022
f22673d
format code
heldersepu Aug 24, 2022
ac7e601
Add check pending owner resets on force transfer
heldersepu Aug 24, 2022
b781427
Update Ownable2Step.test.js
heldersepu Aug 24, 2022
81c1f6c
Update contracts/access/Ownable2Step.sol
heldersepu Aug 29, 2022
a36cc8e
Update contracts/access/Ownable2Step.sol
heldersepu Aug 30, 2022
b0e363a
Update contracts/access/Ownable2Step.sol
heldersepu Aug 30, 2022
8ff0621
Add previousOwner to event OwnershipTransferStarted
heldersepu Aug 30, 2022
ff44765
Merge branch 'master' into master
heldersepu Aug 31, 2022
84f3b9f
Remove forceTransferOwnership
heldersepu Aug 31, 2022
fca0901
Merge branch 'master' into master
frangio Sep 1, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* `Checkpoints`: Use procedural generation to support multiple key/value lengths. ([#3589](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3589))
* `Checkpoints`: Add new lookup mechanisms. ([#3589](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3589))
* `Array`: Add `unsafeAccess` functions that allow reading and writing to an element in a storage array bypassing Solidity's "out-of-bounds" check. ([#3589](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3589))
* `Ownable2Step`: extension of `Ownable` that makes the ownership transfers a two step process. ([#3620](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3620))

### Breaking changes

Expand Down
57 changes: 57 additions & 0 deletions contracts/access/Ownable2Step.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts (last updated v4.7.0) (access/Ownable.sol)

pragma solidity ^0.8.0;

import "./Ownable.sol";

/**
* @dev Contract module which provides access control mechanism, where
* there is an account (an owner) that can be granted exclusive access to
* specific functions.
*
* By default, the owner account will be the one that deploys the contract. This
* can later be changed with {transferOwnership} and {acceptOwnership}.
*
* This module is used through inheritance. It will make available all functions
* from parent (Ownable).
*/
abstract contract Ownable2Step is Ownable {
address private _pendingOwner;

event OwnershipTransferStarted(address indexed previousOwner, address indexed newOwner);

/**
* @dev Returns the address of the pending owner.
*/
function pendingOwner() public view virtual returns (address) {
return _pendingOwner;
}

/**
* @dev Starts the ownership transfer of the contract to a new account. Replaces the pending transfer if there is one.
* Can only be called by the current owner.
*/
function transferOwnership(address newOwner) public virtual override onlyOwner {
_pendingOwner = newOwner;
emit OwnershipTransferStarted(owner(), newOwner);
}

/**
* @dev Transfers ownership of the contract to a new account (`newOwner`) and deletes any pending owner.
* Internal function without access restriction.
*/
function _transferOwnership(address newOwner) internal virtual override {
delete _pendingOwner;
super._transferOwnership(newOwner);
}

/**
* @dev The new owner accepts the ownership transfer.
*/
function acceptOwnership() external {
address sender = _msgSender();
require(pendingOwner() == sender, "Ownable2Step: caller is not the new owner");
_transferOwnership(sender);
}
}
7 changes: 7 additions & 0 deletions contracts/mocks/Ownable2StepMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../access/Ownable2Step.sol";

contract Ownable2StepMock is Ownable2Step {}
57 changes: 57 additions & 0 deletions test/access/Ownable2Step.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
const { constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
const { ZERO_ADDRESS } = constants;
const { expect } = require('chai');

const Ownable2Step = artifacts.require('Ownable2StepMock');

contract('Ownable2Step', function (accounts) {
const [owner, accountA, accountB] = accounts;

beforeEach(async function () {
this.ownable2Step = await Ownable2Step.new({ from: owner });
});

describe('transfer ownership', function () {
it('starting a transfer does not change owner', async function () {
const receipt = await this.ownable2Step.transferOwnership(accountA, { from: owner });
expectEvent(receipt, 'OwnershipTransferStarted', { previousOwner: owner, newOwner: accountA });
expect(await this.ownable2Step.owner()).to.equal(owner);
expect(await this.ownable2Step.pendingOwner()).to.equal(accountA);
});

it('changes owner after transfer', async function () {
await this.ownable2Step.transferOwnership(accountA, { from: owner });
const receipt = await this.ownable2Step.acceptOwnership({ from: accountA });
expectEvent(receipt, 'OwnershipTransferred', { previousOwner: owner, newOwner: accountA });
expect(await this.ownable2Step.owner()).to.equal(accountA);
expect(await this.ownable2Step.pendingOwner()).to.not.equal(accountA);
});

it('changes owner after renouncing ownership', async function () {
await this.ownable2Step.renounceOwnership({ from: owner });
// If renounceOwnership is removed from parent an alternative is needed ...
// without it is difficult to cleanly renounce with the two step process
// see: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3620#discussion_r957930388
expect(await this.ownable2Step.owner()).to.equal(ZERO_ADDRESS);
});

it('pending owner resets after renouncing ownership', async function () {
await this.ownable2Step.transferOwnership(accountA, { from: owner });
expect(await this.ownable2Step.pendingOwner()).to.equal(accountA);
await this.ownable2Step.renounceOwnership({ from: owner });
expect(await this.ownable2Step.pendingOwner()).to.equal(ZERO_ADDRESS);
await expectRevert(
this.ownable2Step.acceptOwnership({ from: accountA }),
'Ownable2Step: caller is not the new owner',
);
});

it('guards transfer against invalid user', async function () {
await this.ownable2Step.transferOwnership(accountA, { from: owner });
await expectRevert(
this.ownable2Step.acceptOwnership({ from: accountB }),
'Ownable2Step: caller is not the new owner',
);
});
});
});