Add comment and tests for zero address behavior in Ownable2Step.trans…

…ferOwnership() (#5226) Co-authored-by: Hadrien Croubois <[email protected]>
OpenZeppelin · Sep 25, 2024 · cc67e0e · cc67e0e
1 parent 2f0bc58
commit cc67e0e
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 0 deletions.
diff --git a/contracts/access/Ownable2Step.sol b/contracts/access/Ownable2Step.sol
@@ -37,6 +37,8 @@ abstract contract Ownable2Step is Ownable {
     /**
      * @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.
+     *
+     * Setting `newOwner` to the zero address is allowed; this can be used to cancel an initiated ownership transfer.
      */
     function transferOwnership(address newOwner) public virtual override onlyOwner {
         _pendingOwner = newOwner;

diff --git a/test/access/Ownable2Step.test.js b/test/access/Ownable2Step.test.js
@@ -81,5 +81,22 @@ describe('Ownable2Step', function () {
 
       expect(await this.ownable2Step.owner()).to.equal(this.accountA);
     });
+
+    it('allows the owner to cancel an initiated ownership transfer by setting newOwner to zero address', async function () {
+      // initiate ownership transfer to accountA
+      await this.ownable2Step.connect(this.owner).transferOwnership(this.accountA);
+      expect(await this.ownable2Step.pendingOwner()).to.equal(this.accountA);
+
+      // cancel the ownership transfer by setting newOwner to zero address
+      await expect(this.ownable2Step.connect(this.owner).transferOwnership(ethers.ZeroAddress))
+        .to.emit(this.ownable2Step, 'OwnershipTransferStarted')
+        .withArgs(this.owner, ethers.ZeroAddress);
+      expect(await this.ownable2Step.pendingOwner()).to.equal(ethers.ZeroAddress);
+
+      // verify that accountA cannot accept ownership anymore
+      await expect(this.ownable2Step.connect(this.accountA).acceptOwnership())
+        .to.be.revertedWithCustomError(this.ownable2Step, 'OwnableUnauthorizedAccount')
+        .withArgs(this.accountA);
+    });
   });
 });