[Ownable] Grant and claim ownership

[mktcode]

Motivation:
I was just contemplating... :)

Details:
I suggest implementing an optional intermediary step when transferring ownership and finalizing it only when claimed by the new owner. Until then the granting can be revoked by the owner.

address private _grantedOwner;

event OwnershipGranted(address indexed grantedOwner);

/**
 * @dev Grants ownership of the contract to a new account (`newOwner`).
 * Ownership has to be claimed by the new account and granting can be
 * revoked until then (`grantOwnership(address(0))`).
 * Can only be called by the current owner.
 */
function grantOwnership(address newOwner) public virtual onlyOwner {
    emit OwnershipGranted(newOwner);
    _grantedOwner = newOwner;
}

/**
 * @dev Claims granted ownership of the contract for a new account (`_grantedOwner`).
 * Can only be called by the currently granted owner.
 */
function claimOwnership() public virtual {
    require(_grantedOwner == _msgSender(), "Ownable: caller is not the granted owner");
    emit OwnershipTransferred(_owner, _grantedOwner);
    _owner = _grantedOwner;
    _grantedOwner = address(0);
}

[mktcode]

Maybe as per default claiming ownership could at the same time grant the original owner to claim it back and new owner has to explicitly revoke that granting by setting it to zero address.

[abcoathup]

Hi @mktcode ! Thanks for the suggestion, it is really appreciated.

The project owner will review your suggestion as soon as they can.

Please wait until we have discussed this idea before writing any code or submitting a Pull Request, so we can go through the design beforehand. We don’t want you to waste your time!

[abcoathup]

OpenZeppelin Contracts has previously included a Claimable, see some of the discussions on including a Claimable again: #1488

[mktcode]

Oh ok, don't know why I didn't find it. Thanks. I'll read it.

[Ro5s]

I think Claimable.sol is a very useful pattern. (1) SECURITY: It allows the transfer of material function controls to a provably functional account. (2) LEGAL: It represents an on-chain "offer" and "acceptance" and formation of a legal contract that defensibly transfers smart contract ctrl. rights (reference to a related value transfer would help this argument as "consideration," though might see such value in the payment of network fees and transfer of liabilities. On this front, making the claim ownership function payable and direct msg.value to transferring owner would be neat).

[frangio]

We removed Claimable at the time when we introduced roles for more granular access control. We left Ownable because it's still useful for quick and dirty tests, but we think most contracts should use the granular control provided by AccessControl.

We considered making AccessControl roles claimable, but in the end we settled for a design that doesn't have a transfer function for roles. There is only grant and revoke which can be invoked by admins, aside from renounce. It's hard for me to see where the grant and claim pattern would make sense in this design.

I would love to hear everyone's thoughts on this though, I may be mistaken. The people that want to use Claimable, have you seen AccessControl? Have you decided against it for your own projects? Please share your experience!

[mktcode]

Well, for me, a two-step ownership transfer just seemed sensible and widely useful enough to be made available in Ownable as a "good for most usecases" snippet. I'd like to have that functionality without dealing with a more "complex" role based access control model.

[heldersepu]

... just came across this after I submitted my PR: #3620
in that I implemented a two-step ownership transfer inheriting from the Ownable contract

[heldersepu]

And I do agree with @mktcode, a two-step ownership transfer is a great addition ...
@frangio I have seen AccessControl and we use, it but for other more complex cases.

[heldersepu]

On the Claimable topic I think the name is a bit vague, "fraud insurance claims" is what came to mind when I read it now for the first time ... I'm all for the least invasive approach, adding grantOwnership and claimOwnership as initially suggested by @mktcode to the existing Ownable sounds good, but still leaves the door open for someone to accidentally paste the wrong value in the old transfer function and gone; in my approach I opted for a new contract, it inherits from the trusted Ownable but I override the transferOwnership two follow the two step process