Add Ownable2Step extension with 2-step transfer (OpenZeppelin#3620)

Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: Francisco <[email protected]>

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
 

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);
+    }
+}

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 {}

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',
+      );
+    });
+  });
+});