# A more secure ownership transfer pattern

Basically, transferring ownership of your contract, e.g. when transferring governance rights or even changing your proxy admin, it's fucking scary. If you transfer the rights to the wrong account, you are most likely losing the ownership forever.

What if we could make the process of transferring ownership more secure?

## Current state

First, let's take a look to how transferring ownership looks like now (src: [ openzeppelin-contracts/contracts/access/Ownable.sol](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/3dac7bbed7b4c0dbf504180c33e8ed8e350b93eb/contracts/access/Ownable.sol)):

    // From openzeppelin-contracts/contracts/access/Ownable.sol (v4.7.0)

    address private _owner;

    // ...
    
    /**
     * @dev Transfers ownership of the contract to a new account (`newOwner`).
     * Can only be called by the current owner.
     */
    function transferOwnership(address newOwner) public virtual onlyOwner {
        require(newOwner != address(0), "Ownable: new owner is the zero address");
        _transferOwnership(newOwner);
    }

    /**
     * @dev Transfers ownership of the contract to a new account (`newOwner`).
     * Internal function without access restriction.
     */
    function _transferOwnership(address newOwner) internal virtual {
        address oldOwner = _owner;
        _owner = newOwner;
        emit OwnershipTransferred(oldOwner, newOwner);
    }

As you can see, you must own the contract and call `transferOwnership`, that's it. The ownership is transferred to the address passed as parameter right away.

## Proposed improvement

Imagine if instead of having a `transferOwnership` that does the transfer in a single step, we would have a two step process, adding a `confirmOwnershipTransfer` function.

So, when you do a `transferOwnership` you won't be transferring it but just queuing the transfer.

Then the new owner candidate needs to explicitly call `confirmOwnershipTransfer`, which will revert in case of not being called by him, in order to accept receiving the ownership rights.

This means that if you were transferring rights to the wrong account, you will realize it when failing at trying to confirm it with the expected new owner and, if that's the case, you can call `transferOwnership` again overriding the new owner candidate (you can even use the current owner as candidate if you want to completely cancel the transfer operation).

Here is how the flow described above could look like:

    // Modification over openzeppelin-contracts/contracts/access/Ownable.sol (v4.7.0)

    address private _owner;
    address private _ownerCandidate;

    // ...

    /**
     * @dev Enqueues the ownership transfer of the contract to a new account (`newOwnerCandidate`).
     * Can only be called by the current owner.
     */
    function transferOwnership(address newOwnerCandidate) public virtual onlyOwner {
        _ownerCandidate = newOwnerCandidate;
    }

    /**
     * @dev Confirms the ownership transfer of the contract to a new account.
     * Can only be called by the new owner candidate.
     */
    function confirmOwnershipTransfer() public virtual {
        require(_ownerCandidate == _msgSender(), "Ownable: caller is not the owner candidate");
        _transferOwnership(_ownerCandidate);
    }

## Going a bit further

If we want extra security, we could even require both the new owner candidate and the current owner to call `confirmOwnershipTransfer`.

At this point you might think I love adding extra steps to processes, but hear me out, this extra step prevents the mistake of performing a `transferOwnership` to a wrong address that was active and executed the `confirmOwnershipTransfer` to accept the erroneous transfer before you override the candidate with a correct one.

Again, here you have an implementation of the described above:

    // Modification over openzeppelin-contracts/contracts/access/Ownable.sol (v4.7.0)

    bool private _transferConfirmedByOwnerCandidate;
    address private _owner;
    address private _ownerCandidate;

    // ...

    /**
     * @dev Enqueues the ownership transfer of the contract to a new account (`newOwnerCandidate`).
     * Can only be called by the current owner.
     */
    function transferOwnership(address newOwnerCandidate) public virtual onlyOwner {
        _ownerCandidate = newOwnerCandidate;
    }

    /**
     * @dev Confirms the ownership transfer of the contract to a new account.
     * Must be first called by the owner candidate and then by the current owner.
     */
    function confirmOwnershipTransfer() public virtual {
        if (_transferConfirmedByOwnerCandidate) {
            require(_owner == _msgSender(), "Ownable: caller is not the owner");
            _transferConfirmedByOwnerCandidate = false;
            _transferOwnership(_ownerCandidate);
        } else {
            require(_ownerCandidate == _msgSender(), "Ownable: caller is not the owner candidate");
            _transferConfirmedByOwnerCandidate = true;
        }
    }

## Conclusion

I personally think this will make the ownership transfer process much secure. Plese, let me know if you agree with the descirbed changes or not, I would love to hear your view.