Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ERC: Owned Standard #173

Closed
danfinlay opened this issue Nov 7, 2016 · 47 comments
Closed

ERC: Owned Standard #173

danfinlay opened this issue Nov 7, 2016 · 47 comments
Labels

Comments

@danfinlay
Copy link
Contributor

danfinlay commented Nov 7, 2016

Opening discussion to nail down semantics for a common pattern that should be standardized.

Many smart-contracts have adopted some pattern of representing the ownership of that contract, and it's such a common and simple pattern, that the exact phrasing should be standardized into an Application-layer protocol, to allow leveraging cross-compatibility.

This standard could be called the Owned standard. I've also heard it called the Asset standard. The standard defines at least one method, either:

  • setOwner(address _newOwner)
  • transferOwner(address _newOwner)

This might be paired with an event, maybe called OwnerTransfer.

It would also probably benefit from having a support method, either one of:

  • getOwner() returns address
  • owner() returns address
  • isOwner(address _owner) returns bool

Places I've seen this used

  • AuctionHouse, a Dapp that allows users to auction any on-chain asset adhering to their own Asset protocol.
  • The proposed Proxy Standard includes a loose implementation of this standard.
  • The Dapple DSAuthorized class essentially implements this pattern.

You can read a more formally specified version of this proposal here:
https://github.com/ethereum/EIPs/blob/master/EIPS/eip-173.md

@apmilen
Copy link

apmilen commented Nov 7, 2016

could transferOwner be replaced with setOwner ? Then it would be nicely paired with getOwner

@danfinlay
Copy link
Contributor Author

I like it. I added it as the new first option.

@SilentCicero
Copy link

@FlySwatter this is super needed, nice, we do need standards across the board for this.

@dob
Copy link

dob commented Nov 8, 2016

Author of the AuctionHouse Asset protocol here. I wanted to point out that it's a little different then what is proposed here. The idea behind the Asset protocol was not that the contract itself was owned or transfered, but that the records tracked within the contract, identified by string _recordId, were owned or transferred.

So for example, a contract:

contract VirtualGoodPlatform is Asset {
  struct VirtualGood {
    address _owner;
    ...;
  }
  mapping(string => VirtualGood) registry;   // Maps the record ID to the good
}

getOwner(_recordId) and setOwner(_recordId, _newOwnerAddress) would update the registry, and not actually update ownership of the contract.

I think settling on a standard for the above is important so that we can have common implementations of trading, buying, selling, and transferring non-fungible assets. But it may be a little different than what you're suggesting here in terms of contract ownership itself. Yes?

@danfinlay
Copy link
Contributor Author

Yes, I guess so!

On Nov 8, 2016, at 5:21 AM, Doug Petkanics notifications@github.com wrote:

Author of the AuctionHouse Asset protocol here. I wanted to point out that it's a little different then what is proposed here. The idea behind the Asset protocol was not that the contract itself was owned or transfered, but that the records tracked within the contract, identified by string _recordId, were owned or transferred.

So for example, a contract:

contract VirtualGoodPlatform is Asset {
struct VirtualGood {
address _owner;
...;
}
mapping(string => VirtualGood) registry; // Maps the record ID to the good
}
getOwner(_recordId) and setOwner(_recordId, _newOwnerAddress) would update the registry, and not actually update ownership of the contract.

I think settling on a standard for the above is important so that we can have common implementations of trading, buying, selling, and transferring non-fungible assets. But it may be a little different than what you're suggesting here in terms of contract ownership itself. Yes?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@Smithgift
Copy link

I suggest using only owner() to get the current owner. If you don't need anything special in determining ownership, you can just have address public owner; and Solidity will happily generate a properly-named getter.

@danfinlay
Copy link
Contributor Author

I agree, it would be good to leverage a public variable for this. Any opinion on the setter name?

@Arachnid
Copy link
Contributor

Shameless plug: ENS can make this pattern obsolete; a contract can do a lookup in the ENS registry for the owner of a given node, and treat that as the owner of the current contract.

@cholewa1992
Copy link

My suggestion is

pragma solidity ^0.4.0;
contract Owned {

    address public owner;

    function Owned() {
        owner = msg.sender;
    }

    modifier isOwner {
        if (msg.sender == owner) _; else throw;
    }

    function transferOwnership(address _newOwner) isOwner {
        owner = _newOwner;
    }
}

I use it for a marketplace structure that I'm working on

@danfinlay
Copy link
Contributor Author

@cholewa1992 The problem I see with that pattern is that you can't easily ask if an arbitrary user is the owner, only if the current user is.

That's why a simple getter (as provided by a public variable) is nice, it's very open ended, and lets the contract speak for itself.

@Arachnid Sorry for not exploring that sooner; So you're saying ENS could make this pattern obsolete if the current owner is registered on ENS, right?

In even that case, the setOwner method would still be useful, and getOwner might still be useful if the contract isn't on ENS.

@Arachnid
Copy link
Contributor

Arachnid commented Jan 5, 2017

Sorry for not exploring that sooner; So you're saying ENS could make this pattern obsolete if the current owner is registered on ENS, right?

In even that case, the setOwner method would still be useful, and getOwner might still be useful if the contract isn't on ENS.

The idea is that you would look up a predetermined name, and only allow the owner of that name to interact with the contract. There's no need to register anyone; just to set an ENS record.

More generally, it's worth considering extending this model to ACLs in general, not just 'owner'.

@nmushegian
Copy link

nmushegian commented Feb 7, 2017

@FlySwatter the multi-user case should be solved by abstraction in the owner, not by complicating the base class.

Also want to make a comment regarding your examples that DSAuth and the dappsys Authority pattern implements different semantics from Owned and shouldn't be used as inspiration for this standard IMO. Take a look at the most recent version to see how we collapsed the "owner" concept inside DSAuth entirely into the authority, getting out of the way of existing interfaces. We do not want to have a shared ABI with this.

I also strongly favor a minimal variant like the one @cholewa1992 suggested.

@danfinlay
Copy link
Contributor Author

I think this pattern has been largely canonized in the DS-Auth pattern.

@tymat
Copy link

tymat commented Feb 14, 2017

We are using this for DigixCore 2.0 Assets with an additional mint() function which converts the assets into ERC-20 tokens.

@nmushegian
Copy link

+1 to mint and its dual burn, but let's keep that thread separate

@mudgen
Copy link
Contributor

mudgen commented Jun 6, 2018

@Arachnid It seems that your suggestion to use ENS would only solve one of the problems.
Here's the problems to solve:

  1. Contract A needs to authenticate if user is the owner of the contract or not.
  2. Contract B wants to find out who is the owner of Contract A.

It seems that ENS could be used to authenticate that a user is owner of a contract or not.

But it seems that ENS could not be used in Contract B to get the owner address of Contract A. Correct? So a function such as owner() returns (address) would still be needed in Contract A, right?

@Arachnid
Copy link
Contributor

Arachnid commented Jun 6, 2018

@mudgen If contract A determines its owner by looking up an ENS record, contract B can do the same.

@mudgen
Copy link
Contributor

mudgen commented Jun 6, 2018

@Arachnid, if contract B only has the ethereum address of contract A, then how can it get the owner address of contract A?

Let's say that Contract A calls a function in Contract B. The function in Contract B wants to get the owner address of Contract A but it only has the ethereum address of Contract A via msg.sender. Could ENS still be used to get the owner address of Contract A?

@mudgen
Copy link
Contributor

mudgen commented Jun 6, 2018

I can see some users wanting to use an owner() function and some users also wanting to use transferOwner(). So I think it might be a good idea to have two interfaces, one just for owner() and another one just for transferOwner() so that users can choose either one or both and still be compliant with the standard.

I think it is important to have an agreed upon way to get the owning address of a contract. Having this will help interoperability between contracts and other software.

@danfinlay Are you planning to move this issue through the standards process? If not, is it okay with you if I create a draft EIP about this and move the Owned Standard forward?

@MicahZoltu
Copy link
Contributor

@Arachnid I'm not a fan of using ENS for this due to gas costs. A cross-contract ENS check of ownership on a function modified with onlyOwner is significantly more expensive (relatively) than just checking a local storage variable. Unless we can get cross contract calls down, there are a number of things that we could move off contract but must consider the gas costs of doing so.

@MicahZoltu
Copy link
Contributor

Recommend checking out https://github.com/OpenZeppelin/openzeppelin-solidity/tree/master/contracts/ownership for a bunch of things related to ownership. One pattern I am a big fan of is Claimable rather than Ownable. It makes it so it is impossible to accidentally transfer ownership to /dev/null. It does mean that ownership transfers are a two step process, but if we assume the usage pattern of ownership transferring is "infrequent" then I don't think that matters as much as the security benefits of pending owner.

@MicahZoltu
Copy link
Contributor

Something I don't see discussed here is why this needs to be standardized rather than just a best practice? In the original description @danfinlay says:

should be standardized into an Application-layer protocol, to allow leveraging cross-compatibility.

Can you go into detail on how we could leverage standardization for cross-compatibility? Is there an opportunity for shared tooling that works with Ownable contracts?

My gut tells me that the opportunity for shared tooling is pretty limited for ownable contracts, and I would rather see effort spent on something like #719 that would allow us to have shared tooling for good UX of arbitrary contracts than trying to build one-off shared tooling for every common pattern.

@mudgen
Copy link
Contributor

mudgen commented Jun 7, 2018

@MicahZoltu I am not an implementer of contract UI tools so I am not going to give examples of that. I am an implementer of contracts so I will give some examples of how an Owned Standard would be very useful for controlling ownership of contracts with contracts.

  1. Exchanges that buy/sell/auction ethereum contracts. This would only be possible if there was a standard for specifying a way to get the owning address of a contract and a standard way to transfer that ownership.
  2. Contract wallets that hold the ownership of contracts and that can transfer the ownership of contracts.
  3. Contract registries. It makes sense for some registries to only allow the owners of contracts to add their contracts to registries. A standard must exist for these contract registries to verify that a contract is being submitted by the owner of it before accepting it.

The Owned Standard popped up on my radar because I need to create a simple contract registry for a system I want to make. But for it to work with more contracts there needs to be a standard way to get the ownership of a contract.

@danfinlay
Copy link
Contributor Author

I don't have a specific use case in mind, so I'm not that interested in defending this proposal anymore, but I will briefly anyways:

I understand https://github.com/ProjectWyvern is a protocol for selling owned smart contracts, so they would be a good example of a smart contract that would benefit from common and widespread interfaces between ownable smart contracts.

I'm a huge fan of 719, but that's more about client rendering, this proposal would allow inter-operability between smart contracts that are owned, and allow other smart contracts to more easily manage their ownerships.

@mudgen
Copy link
Contributor

mudgen commented Jun 7, 2018

I wrote an EIP about this here: https://github.com/mudgen/EIPs/blob/master/EIPS/eip-173.md and submitted a pull request.

The first draft is ready for some feedback.

@mudgen
Copy link
Contributor

mudgen commented Jun 8, 2018

EIP 173 has been merged and is here: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-173.md

Please give feedback.

@mudgen
Copy link
Contributor

mudgen commented Jun 8, 2018

@danfinlay Can you edit your first post in this issue to add a link to the proposed standard here: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-173.md

@androlo
Copy link

androlo commented Jun 8, 2018

@MicahZoltu Absolutely agree. Forcing the new owner to claim ownership before the transfer is an essential step. It ensures that the new owner is an account that is at least able to transact with the contract, which means the contract can't be locked down indefinitely due to a typo, or bugs like encoding errors.

I recently wrote an article on the "ownable" and "claimable" patterns, if anyone is interested: https://www.ohalo.co/blog/redundancy-is-needed-when-building-safe-smart-contracts

@mudgen
Copy link
Contributor

mudgen commented Jun 8, 2018

@MicahZoltu @androlo do you think the standard should support both patterns: claimable and transfer?

@shrugs
Copy link

shrugs commented Jun 8, 2018

How necessary do you think the owner() getter is? We brought this up in OpenZeppelin chat, but, for example, RBACOwnable from https://github.com/OpenZeppelin/openzeppelin-solidity/pull/950/files#diff-8f869785a4b22b507bae87a6dbf3f2eb couldn't conform to this standard, since it doesn't store the address of the owner in an easily gettable way (as it supports multiple owners).

Any thoughts on whether or not RBACOwnable should or could conform to this standard?

@androlo
Copy link

androlo commented Jun 8, 2018

@mudgen I don't know. Maybe it is good to do like zeppelin and create two separate interfaces.

@fulldecent
Copy link
Contributor

fulldecent commented Jun 9, 2018

Presently the ownership of a contract seems useful. Metamask and etherscan could show this data.

I'm not yet 100% sure the transfer mechanism needs to be standardized. Standardized as in, will Metamask and etherscan ever call that function?

Assuming yes for the following notes.

The one-step transfer function makes sense. The documentation is broken, can be fixed.

The two-step transfer function DOES NOT NEED TO BE STANDARDIZED. So I like the current version. If not clear, I can provide detail here.

@shrugs
Copy link

shrugs commented Jun 9, 2018

Here are the various standard interfaces I could see coming out of this:

  1. "who has this permission over this contract"
  2. "does this specific account have permission over this contract"
  3. "in a single-owner context, this is how you transfer permission without confirmation"
  4. "in a single-owner context, this is how you transfer permissions with confirmation"
  5. "in a multi-owner context, this is how you add and remove permission"

@xinbenlv
Copy link
Contributor

Suggest adding requires: 165 to the summary

@mudgen
Copy link
Contributor

mudgen commented Jul 20, 2020

@danfinlay and I are finally pushing this standard through the process to become final. Two questions for you:

  1. Do you have any more feedback for this standard?
  2. Do you know of more implementations of the standard? If so please provide a link. I'd like to add more links to implementations of the standard so people get more of an idea of where it is currently being used.

The current version of the standard is here: https://eips.ethereum.org/EIPS/eip-173

Thanks!

@ethereum ethereum deleted a comment from mohsag Jul 21, 2020
@ethereum ethereum deleted a comment from mohsag Jul 21, 2020
@fulldecent
Copy link
Contributor

  • Snippet should have solidity version
  • transferOwnership function should support payable, see arguments on ERC-721

@mudgen @danfinlay

@Amxx
Copy link
Contributor

Amxx commented Aug 2, 2021

The review period is supposed to have ended almost a year ago. Shouldn't this be marked final ?

@axic
Copy link
Member

axic commented Sep 21, 2021

I am not sure what implementations of this standard exists, but I would bet a very large proportion of contracts use OpenZeppelin's implementation.

That OpenZeppelin implementation has one crucial difference: transferOwnership cannot transfer to address(0), but instead there's a new function called renounceOwnership to accomplish that.

The standard prescribes:

    /// @notice Set the address of the new owner of the contract
    /// @dev Set _newOwner to address(0) to renounce any ownership.
    /// @param _newOwner The address of the new owner of the contract    
    function transferOwnership(address _newOwner) external;

In my interpretation OZ is compatible with it, as implementations can impose additional restrictions. However my question goes to @Amxx and @frangio: what is the reasoning behind this?

@Amxx
Copy link
Contributor

Amxx commented Sep 22, 2021

@axic It was felt that splitting the transfer and the renounce would reduce the risks of mistakes ... which is debatable since you can transfer to 0x1 or 0xdead ...

At this point we keep this design for backward compatibility.

@fulldecent
Copy link
Contributor

The solution is 0xcert's implementation. That allows you to nominate a new owner and then claim ownership from that account.

I'm not really sure what the goal here is to standardize because I don't see a big demand for consumers for this interface.

@Amxx
Copy link
Contributor

Amxx commented Sep 22, 2021

@fulldecent What you are talking about is a completelly different pattern/workflow.

@axic
Copy link
Member

axic commented Sep 22, 2021

I'm not really sure what the goal here is to standardize because I don't see a big demand for consumers for this interface.

I think the benefit of standardising is that contracts are queryable using ERC-165 to see if these are supported. It would be nice for wallets/clients/tools to display what standards contracts support at the time of interaction.

@drllau
Copy link

drllau commented Oct 30, 2021

@danfinlay and I are finally pushing this standard through the process to become final. Two questions for you:
2. Do you know of more implementations of the standard? If so please provide a link. I'd like to add more links to implementations of the standard so people get more of an idea of where it is currently being used.

LexDAO has a contract suite (conditional payments), one of which condition is ownable. The only significant difference is that the transfer function can be called from either the previousOwner or the newOwner

    function transferOwner(address to, bool direct) external onlyOwner {
        require(to != address(0), "ZERO_ADDRESS");

        if (direct) {
            owner = to;
            emit TransferOwner(msg.sender, to);
        } else {
            pendingOwner = to;
            emit TransferOwnerClaim(msg.sender, to);
        }
    }`

This may be worked around by conforming to final standard and add in ERC1363 to make arbitrary call on recipient after transfer.

@drllau
Copy link

drllau commented Oct 30, 2021

The issue this raises is whether the transfer of ownership can be rejected by the recipient. From a judicial perspective (hey, we're a guild of legal enginners) a "gift" (arbitrary transfer without consideration) needs 3 elements

  1. owner shows intention to give
  2. delivery of gift
  3. action showing recipient acceptance (cf random package left on doorsteps == abandoned)

There may be some (tax) paperwork if gifts exceed a threshold, hence the callback which comes in useful in modifying the acceptance. Also what will the new account abstraction do when the gas is associated with the contract and not EOA? You may potentially end up with dead contracts sitting in Merkle tree which still has a modicum of useful gas but not enough to do anything.

@z0r0z
Copy link
Contributor

z0r0z commented Nov 5, 2021

maybe we can consider removing ERC-165 introspection to simplify this module? Essentially, I am not sure the benefit of ERC-165 where support for interfaces can be spoofed.

@github-actions
Copy link

github-actions bot commented May 4, 2022

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label May 4, 2022
@github-actions
Copy link

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests