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

Interfaces can't inherit #3419

Closed
fulldecent opened this issue Jan 21, 2018 · 35 comments · Fixed by #8091
Closed

Interfaces can't inherit #3419

fulldecent opened this issue Jan 21, 2018 · 35 comments · Fixed by #8091
Labels
language design :rage4: Any changes to the language, e.g. new features

Comments

@fulldecent
Copy link
Contributor

fulldecent commented Jan 21, 2018

The following fails to compile. I make a case here that it should.

pragma solidity ^0.5;

interface I1 {
    function a() external;
}

// ✅ Test 1 -- should pass 
interface I2 is I1 {
    function b() external;
}

// ✅ Test 2 -- should pass 
interface I3 is I1 {
    function a() external override(A);
}

Why?

Best practice now is for ERCs to publish a Solidity interface. This defines a standard for compliant contracts to implement.

Examples: ERC-721, ERC-777, ERC-1155, ERC-875 (DRAFT). (This is every Final and Last Call ERC since 721.)

Very often (most of the above examples) the standards author will specify that implementing a standard requires implementing another standard as a prerequisite. For example to implement ERC-721 it is specified that you must implement ERC-165. To implement the ERC-721 Metadata Extension it is specified that you must first implement ERC-721.

Currently, authors are expressing this in code like:

/// @title ERC-721 Non-Fungible Token Standard
/// @dev See https://eips.ethereum.org/EIPS/eip-721
///  Note: the ERC-165 identifier for this interface is 0x80ac58cd.
interface ERC721 /* is ERC165 */ {
    // ...
}

This can be adopted formatting in the language.

Related

@axic
Copy link
Member

axic commented Jan 21, 2018

Why would it need to extend another interface? A contract can inherit multiple interfaces.

@fulldecent
Copy link
Contributor Author

Here's why:

pragma solidity ^0.4.19;

interface A {
    uint32[] public contracts;
}

interface B is A {
    function lastContract() public returns (uint32 _contract);
}

Interface B depends on A. This fact should be encodable in B, and it can be done with inheritance.

@fulldecent
Copy link
Contributor Author

When you are writing ERC20Equity you will want to be sure it complies with the ERC20 interface. And that should be enforced and checked by the compiler.

@ondratra
Copy link

ondratra commented Apr 5, 2018

Why is this still not solved? This is must have for interfaces. Can I help with implementation or what's the status on this?

@fulldecent
Copy link
Contributor Author

Because I’m not a paid employee.

Please see the discussion on reinventing inheritance in the other issue. And we are reviewing the finer details of this change. On cell phone, sorry no link.

@ondratra
Copy link

ondratra commented Apr 5, 2018

Not sure what is current issue i should check, can you send me link?

I think this "interface composition" could be implemented right away without changes seen here. I am willing to help with it.

@chriseth
Copy link
Contributor

chriseth commented Apr 6, 2018

@ondratra thank you for your offer, but the implementation is not the issue. The problem is that we want to properly fix inheritance and function overloading, and this requires some discussion. Please take a look here: #3729

@axic axic added the language design :rage4: Any changes to the language, e.g. new features label Jul 28, 2018
@branaway
Copy link

branaway commented Sep 1, 2018

Desperately need interface inheritance for clean OOD!

@k06a
Copy link

k06a commented Oct 15, 2018

Interfaces are unusable until inheritance implemented.

@fulldecent
Copy link
Contributor Author

The current workaround is to use:

/* interface */ contract ERC721 {
  ...
}

I hear you loud and clear. Progress to fix this issue is happening at #3729

@nfurfaro
Copy link

It seems like an interface should stand on it's own, without needing to inherit. A collection of small, specific interfaces can be composed by a contract which inherits them. I don't think an interface should depend on anything else.

@fulldecent
Copy link
Contributor Author

@nfurfaro I disagree.

The intent of the meaning "Interface A inherits interface B" can also be stated as "interface A requires interface B". This is a common concept and the compiler should understand it.

Here is just one example:

https://github.com/0xcert/ethereum-erc721/blob/master/contracts/tokens/ERC721Metadata.sol is an interface with three functions. It is specified in documentation that using this interface only makes sense if you also implement the interface https://github.com/0xcert/ethereum-erc721/blob/master/contracts/tokens/ERC721.sol

The former literally means nothing without the latter.

@k06a
Copy link

k06a commented Oct 22, 2018

@fulldecent you're right, but I prefer "interface A extends interface B". I OOP meaning, interface A is more specific than interface B.

@nfurfaro
Copy link

@fulldecent I see your point. Can this be achieved using Abstract contracts? Use a single Abstract contract as a base contract for the implementation, and the Abstract contract can inherit as many interfaces as needed. I realize this doesn't quite codify the relationship between the 2 interfaces in your example in the same way that interface inheritance would.

@nfurfaro
Copy link

nfurfaro commented Oct 22, 2018 via email

@fulldecent
Copy link
Contributor Author

Yes, an abstract contract or a contract is able to achieve the same thing. But semantics are lost.

Semantically, inter-contract communication is described by interfaces (which represent Ethereum ABI). If people that are standardizing inter-contract communication (🤚) are using interfaces that implement or extend other interfaces then this behavior is more semantic to be in interfaces.

@nfurfaro
Copy link

nfurfaro commented Oct 23, 2018 via email

@fulldecent
Copy link
Contributor Author

Issue updated and references added per weekly meeting today.

@axic
Copy link
Member

axic commented Dec 12, 2019

Didn't we agree on the lat meeting that we should allow interfaces to inherit to signal dependency, but they cannot override any specifications?

@chriseth
Copy link
Contributor

@axic sounds good!

@fulldecent
Copy link
Contributor Author

@axic Yes. Sorry for the omission. I have corrected the test cases in the top comment.

@wjmelements
Copy link
Contributor

I'm glad to see this will finally make it into solidity.

@k06a
Copy link

k06a commented Dec 18, 2019

Hope to see this “interface extending” feature soon, to avoid using “abstract contracts” for this purpose

@nfurfaro
Copy link

nfurfaro commented Dec 18, 2019 via email

@randomnetcat
Copy link
Contributor

randomnetcat commented Dec 20, 2019

Another ambiguous test case:

interface A {
    function test() external returns (uint256);
}

interface B {
    function test() external returns (uint256);
}

// Accepted or error?
interface Sub is A, B {}

@axic
Copy link
Member

axic commented Dec 20, 2019

@random-internet-cat good catch. I don't think we should use the current inheritance rules in this case. It should be an error if there are conflicting definitions in the interfaces.

@randomnetcat randomnetcat mentioned this issue Jan 2, 2020
6 tasks
@leonardoalt
Copy link
Member

leonardoalt commented Jan 4, 2020

@axic @random-internet-cat The current inheritance rules do issue an error for this case (with abstract contract instead of interface), and I think it should be an error there too:
Error: Derived contract must override function "test". Two or more base classes define function with same name and parameter types.

@chriseth
Copy link
Contributor

chriseth commented Jan 8, 2020

@axic I disagree. We should provide a means to say "yes, these two functions in the interface actually mean the same thing". The following is legal:

pragma solidity ^0.6.0;
interface A {
    function test() external returns (uint256);
}

interface B {
    function test() external returns (uint256);
}

abstract contract Sub is A, B {
    function test() external virtual override(A,B) returns (uint256);
}

And thus this should also be legal (note no "virtual"):

pragma solidity ^0.6.0;
interface A {
    function test() external returns (uint256);
}

interface B {
    function test() external returns (uint256);
}

interface Sub is A, B {
    function test() external override(A,B) returns (uint256);
}

@ekpyron
Copy link
Member

ekpyron commented Jan 14, 2020

I fully agree with @chriseth on this one.
Overriding with unimplemented functions was added exactly for this purpose - it actually makes even more sense for interfaces than for abstract contracts.
I'd say we spent quite a lot of effort to design the contract inheritance rules to nicely deal with all cases like this, so I'm pretty sure we can and should use the very same rules for interfaces.

@randomnetcat
Copy link
Contributor

So the contract labeled as "Test 2" in the original message should be permitted, since it would be permitted under the usual inheritance rules?

@ekpyron
Copy link
Member

ekpyron commented Jan 15, 2020

@random-internet-cat That's probably the one thing that needs discussion.
I myself would say: yes, "Test 2" is valid (of course only with an explicit override specifier). But I'm not sure everyone agrees on that.

I'd argue that we could indeed disallow all cases of "useless overriding with unimplemented functions" (i.e. it's only valid to override with unimplemented functions, if there are multiple ambiguous base functions), but I'm not sure about that and even if we want to, then I'd do that not only for interfaces, but also for (abstract) contracts (which would strictly speaking be breaking). So I would decouple this from the initial implementation of interface inheritance and consider it a separate issue.
So for now as far as I'm concerned the only difference between contract and interface inheritance should be that interface functions are all implicitly virtual.

@chriseth
Copy link
Contributor

Second that:

"So for now as far as I'm concerned the only difference between contract and interface inheritance should be that interface functions are all implicitly virtual."

@chriseth
Copy link
Contributor

accepted the above in the meeting.

@fulldecent
Copy link
Contributor Author

Thank you. Test case at top is updated.

kophyo1234 added a commit to kophyo1234/openzeppelin-contracts that referenced this issue Jan 29, 2022
Standards

ERC-20 Token Standard.
ERC-165 Standard Interface Detection.
ERC-173 Owned Standard.
ERC-223 Token Standard.
ERC-677 transferAndCall Token Standard.
ERC-827 Token Standard.
Ethereum Name Service (ENS). https://ens.domains
Instagram – What’s the Image Resolution? https://help.instagram.com/1631821640426723
JSON Schema. https://json-schema.org/
Multiaddr. https://github.com/multiformats/multiaddr
RFC 2119 Key words for use in RFCs to Indicate Requirement Levels. https://www.ietf.org/rfc/rfc2119.txt
Issues

The Original ERC-721 Issue. ethereum/EIPs#721
Solidity Issue OpenZeppelin#2330 – Interface Functions are External. ethereum/solidity#2330
Solidity Issue OpenZeppelin#3412 – Implement Interface: Allow Stricter Mutability. ethereum/solidity#3412
Solidity Issue OpenZeppelin#3419 – Interfaces Can’t Inherit. ethereum/solidity#3419
Solidity Issue OpenZeppelin#3494 – Compiler Incorrectly Reasons About the selector Function. ethereum/solidity#3494
Solidity Issue OpenZeppelin#3544 – Cannot Calculate Selector of Function Named transfer. ethereum/solidity#3544
CryptoKitties Bounty Issue OpenZeppelin#4 – Listing all Kitties Owned by a User is O(n^2). dapperlabs/cryptokitties-bounty#4
OpenZeppelin Issue OpenZeppelin#438 – Implementation of approve method violates ERC20 standard. OpenZeppelin#438
Solidity DelegateCallReturnValue Bug. https://solidity.readthedocs.io/en/develop/bugs.html#DelegateCallReturnValue
Discussions

Reddit (announcement of first live discussion). https://www.reddit.com/r/ethereum/comments/7r2ena/friday_119_live_discussion_on_erc_nonfungible/
Gitter #EIPs (announcement of first live discussion). https://gitter.im/ethereum/EIPs?at=5a5f823fb48e8c3566f0a5e7
ERC-721 (announcement of first live discussion). ethereum/EIPs#721 (comment)
ETHDenver 2018. https://ethdenver.com
NFT Implementations and Other Projects

CryptoKitties. https://www.cryptokitties.co
0xcert ERC-721 Token. https://github.com/0xcert/ethereum-erc721
Su Squares. https://tenthousandsu.com
Decentraland. https://decentraland.org
CryptoPunks. https://www.larvalabs.com/cryptopunks
DMarket. https://www.dmarket.io
Enjin Coin. https://enjincoin.io
Ubitquity. https://www.ubitquity.io
Propy. https://tokensale.propy.com
CryptoKitties Deployed Contract. https://etherscan.io/address/0x06012c8cf97bead5deae237070f9587f8e7a266d#code
Su Squares Bug Bounty Program. https://github.com/fulldecent/su-squares-bounty
XXXXERC721. https://github.com/fulldecent/erc721-example
ERC721ExampleDeed. https://github.com/nastassiasachs/ERC721ExampleDeed
Curio Cards. https://mycuriocards.com
Rare Pepe. https://rarepepewallet.com
Auctionhouse Asset Interface. https://github.com/dob/auctionhouse/blob/master/contracts/Asset.sol
OpenZeppelin SafeERC20.sol Implementation. https://github.com/OpenZeppelin/zeppelin-solidity/blob/master/contracts/token/ERC20/SafeERC20.sol
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language design :rage4: Any changes to the language, e.g. new features
Projects
None yet
Development

Successfully merging a pull request may close this issue.