diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f99f35cac3..76d813b1b79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ * `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)) * `Strings`: optimize `toString`. ([#3573](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3573)) + * `Ownable2Step`: extension of `Ownable` that makes the ownership transfers a two step process. ([#3620](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3620)) ### Breaking changes diff --git a/contracts/access/Ownable2Step.sol b/contracts/access/Ownable2Step.sol new file mode 100644 index 00000000000..5ee5cf8c7f6 --- /dev/null +++ b/contracts/access/Ownable2Step.sol @@ -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); + } +} diff --git a/contracts/mocks/Ownable2StepMock.sol b/contracts/mocks/Ownable2StepMock.sol new file mode 100644 index 00000000000..606d0c96436 --- /dev/null +++ b/contracts/mocks/Ownable2StepMock.sol @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../access/Ownable2Step.sol"; + +contract Ownable2StepMock is Ownable2Step {} diff --git a/test/access/Ownable2Step.test.js b/test/access/Ownable2Step.test.js new file mode 100644 index 00000000000..0aeb2246541 --- /dev/null +++ b/test/access/Ownable2Step.test.js @@ -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', + ); + }); + }); +});