ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
23.29k stars 5.77k forks source link

Keyword to declare source code which is inheritable, but not compilable. #5046

Closed maurelian closed 6 years ago

maurelian commented 6 years ago

My most frequent pain point with inheritance is ambiguity about "which of these things are actually being deployed".

A typical repository might have 30 contracts, but only 5 or 6 derived contracts which compile to bytecode that will be deployed. For example, there is no reason to ever deploy a contract which is simply Ownable, so why would that be declared as contract Ownable { ... }.

I was impressed with how the 0xProject handled this with a strict naming convention using the terminology of 'Mixins'. For example, in this directory, only Exchange.sol will be deployed, everything else will be inherited to that contract.

I would like to propose creating a new keyword for declaring an inheritable piece of code. The chief benefit would be a clear indication that "this is not intended to be a final contract".

Perhaps: inheritable, mixin or class, could be used. The specific word isn't too important.

Originally posted by @maurelian in https://github.com/ethereum/solidity/pull/3729#issuecomment-422975059

maurelian commented 6 years ago

@fulldecent I felt like this deserved it's own issue. Feel free to place your abstract proposal in a comment here. For other's it's here: https://github.com/ethereum/solidity/pull/3729#issuecomment-423379028

The term abstract has never made a lot of sense to me in this context, but I've seen it used for precisely this purpose so it might be the best choice. I like the behaviors you proposed as well.

fulldecent commented 6 years ago

Consider the keyword abstract.

Example from https://github.com/0xcert/ethereum-utils/blob/master/contracts/ownership/Ownable.sol

pragma solidity ^0.5.0;

abstract contract Ownable {
  address public owner;

  // event OwnershipTransferred...

  constructor() public {
    owner = msg.sender;
  }

  modifier onlyOwner() {
    require(msg.sender == owner);
    _;
  }

  function transferOwnership(address _newOwner) onlyOwner public {
    // ...
  }
}

Nobody in their right mind would deploy the Ownable contract, even though it is presently possible. The keyword abstract removes the ability to compile this contract. And in the Remix IDE, abstract contracts won't even show up as compile targets.

In addition to allowing abstract keyword to compileable contracts. It is probably helpful to require abstract for non-compilable contracts.

pragma solidity ^0.4.25;

contract A {
    function x();
}

^^^ Presently this code is legal. I propose that it should be an error "Contracts with unimplemented methods must be explicitly marked as abstract." Or better, "Contract A must implement function x() or be marked as abstract". And of course, interfaces are implicitly abstract.

axic commented 6 years ago

There is a long outstanding issue to track this: #649

Is it possible to move this discussion over there?

fulldecent commented 6 years ago

👍 / I'll do one more copy paste

maurelian commented 6 years ago

Dupe of: https://github.com/ethereum/solidity/issues/649