Solidity: Explicit keyword for abstract contracts

[Antanukas]

Compiler doesn't produce errors, but returns empty bytecode when child contract does not call inherited constructor.

To reproduce:

contract parent {
  function parent(address _someValue) {}
}

contract child is parent {
//  Uncomment to fixes compilation
//  function child(address a) parent(a) {}
}

run sold child.sol --bin
output:

======= child =======
Binary: 


======= parent =======
Binary: 
60606040526040516020806038833981016040528080519060200190919050505b5b50600a80602e6000396000f360606040526008565b00

solc version:

solc, the solidity compiler commandline interface
Version: 0.3.4-0/RelWithDebInfo-Linux/g++/Interpreter

Expected: some error message that child must define constructor

[chriseth]

If you do not proved base contract constructor arguments, your contract is abstract, as if you did not implement one of the methods. There is nothing wrong with that, so it is not possible to show any error.

Having said that, we plan to change the way the compiler is invoked so you can explicitly specify which contract you want to compile (instead of all non-abstract ones). In that case, the compiler can provide an error message and should also tell you the reason why a contract is abstract.

[Antanukas]

So maybe a simple keyword 'abstract' could be added like in Java, Scala, C#? This way compiler will have enough information to warn user if his intention was not an abstract contract. Cause I spent some time until I have realized this mistake.

In my concrete case I had a big contract which was inheriting from a contract with constructor and I have missed the constructor in derived contract. This resulted in empty bytecode from compiler which seems incorrect.

[axic]

So right now the only way to test if it is an abstract contract is by seeing if a bytecode is returned during compilation. For real abstract contracts, that is empty.

Fun fact, the below is not abstract:

contract A {
}

because it contains only the implicit definition of the fallback function.

[axic]

I think having an explicit keyword could aid error reporting. Should it be called abstract contract or interface though?

[redsquirrel]

I think it would be _A Very Good Thing_ to have an explicit abstract keyword to denote abstract contracts. It would improve the readability of Solidity code even more than the constant keyword.

@axic I wouldn't use interface because (at least in Java) interfaces can't implement any methods (all methods are implicitly abstract), whereas Solidity's abstract contracts can implement methods, leaving at least one method abstract. (Very similar to Java's abstract classes.)

[axic]

@redsquirrel forgot that it can have implemented as well as non-implemented functions.

I'm not sure abstract contracts make sense in that case, perhaps marking such functions clearly abstract makes more sense?

contract Weird {
  function a() abstract;
  function b() {
    if (msg.value > 0)
       a();
  }
}

I would personally prefer to have a clean interface definition in any case, so that we can have a clean, interface-only Token (and Mango Repository) object to be used for linking. Basically the Solidity source code version of the JSON ABI.

interface WeirdInterface {
  function a();
  function b();
}

[redsquirrel]

@axic In Java, you can mark methods and classes as abstract. For solidity, I recommend requiring an explicit abstract keyword for abstract (un-instantiable) contracts, and letting abstract functions be implicitly abstract.

Seems like the interface idea deserves it's own issue/discussion.

[chriseth]

As also commented on the "interface" issue:

I think having another type of contract is not a good idea and will confuse users. What about:

Warn if contract is abstract but not "marked" as abstract.
You can mark a contract as abstract by using contract a is abstract {}.
It is a bit weird, because abstract would be a keyword that is not inherited, i.e. contracts that derive from a will not necessarily be abstract.

Also having the distinction between explicitly abstract contract and purely abstract (interface) is too fine and confusing. Please let's not go down the path C# went.

[redsquirrel]

Warn if contract is abstract but not "marked" as abstract.

I'd prefer to see it more strictly enforced with an error, but a warning is fine.

You can mark a contract as abstract by using contract a is abstract {}.

I think this is going to confuse people because it looks like inheritance. I'd prefer:

abstract contract a {}

[chriseth]

Yep, that is how it is done in Typescript, so we could also follow that.

[axic]

You can mark a contract as abstract by using contract a is abstract {}.

I think this is a bad idea. It suggests that it is derived from an abstract parent, but that wouldn't mean your contract cannot be instantiated.

Also having the distinction between explicitly abstract contract and purely abstract (interface) is too fine and confusing. Please let's not go down the path C# went.

Abstract contracts are probably useful to map out source code in a more sensible way.

Interfaces on the other hand map the ABI 1-to-1 and have a different use case. It helps to avoid the mistake of using an abstract contract, which has a few extra methods defined, and pointing it to an instance which only has the interface methods available.

Take this example:

contract MyInterface {
    function a() returns (bytes32);
}

contract Abstract is MyInterface {
    function a() returns (bytes32);
    function b() returns (bytes32) {
        return a();
    }
}

contract Real is MyInterface {
    function a() returns (bytes32) {
        return 0x4242;
    }
}

contract Test {
    function a() returns (bytes32) {
        return Abstract(<the address of Real>).b(); // this compiles, but will fail
        return MyInterface(<the address of Real>).b(); // this will raise the compilation error
    }
}

MyInterface is the interface (i.e. Token) and Real is an actual contract implementing that interface (i.e. a real token).

[redsquirrel]

return Abstract(<the address of Real>).b(); // this compiles, but will fail

Why would this fail?

[axic]

Because the instance of Real doesn't implement the method b. Real is derivate of the interface and not the abstract, but when interfaces are defined as abstracts (as today), this can easily be uncaught.

[redsquirrel]

Ahhh, I see it now. I thought that Real inherited from Abstract. Now I see what you were doing. Thanks.

[asinyagin]

I like both abstract contract and interface but the second one is shorter.

[chriseth]

I think we should discuss this again. Currently, there are many situations that lead to a contract being abstract, but you get not notified about that unless you try to instantiate such a contract. If we require a keyword of some sort for such cases, it is much easier to detect. (suggested by @recmo)

[axic]

Since we have interface for interfaces, the only choice here remains (based on the above): abstract contract.

I would suggest an alternative: partial contract.

I'm still a proponent of making this required.

[XertroV]

As a workaround it'd be good to emit a warning on abstract contracts - this allows for the compiler to give feedback to coders who don't expect the sort of thing I talk about in #4220

Part of the reason this happens is b/c function a(bytes16 b) and function a(bytes32 b) have different hashes so solidity considers them different functions that can co-exist. This is VERY different to how most other languages work

Alternate suggestion: a "strict" pragma (or something) that doesn't allow identically named functions with different arguments. This would allow my case (#4220) to be caught and isn't a breaking change.

My feeling is this is a significant issue because it requires the programmer to learn more than should be necessary about the internal functioning of solidity and the compiler, which _should not be the default_. Good compilers give good feedback (erring on the side of more feedback) and _help_ the programmer as much as possible. My 2 wei.

[axic]

As a workaround it'd be good to emit a warning on abstract contracts

I do not think that is a viable approach since we do not have a way to suppress warnings and as a result there would be no way to create abstract contracts without warnings.

[axic]

@chriseth @ekpyron what do you think about partial contract or abstract contract?

[ekpyron]

Between the two I would prefer abstract contract, and I would also prefer this to be an error and not merely a warning.

In general I'd prefer a single keyword for abstract contracts, but since interface is already taken, I'm not sure whether there's a good choice (and arguably having contract, interface and yet another keyword for abstract contracts may be too much - there's even library as well already as a top-level construct). abstract C { ... } doesn't look nice either...

[chriseth]

I think I would prefer contract abstract Name { although I also don't particularly like it.

[ekpyron]

What about contract Name abstract {? That's closer to a modifier. Since we have is for inheritance, contract Name abstract is Base { is still unambiguous. Or one also could consider contract Name is Base abstract {, although that looks stranger to me.
For some reason in post notation I think contract Name partial { looks better than partial contract does, so in that ordering I'd actually be fine with that as well.

[fulldecent]

I consider abstract contract as a type.

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.

[chriseth]

Quick decision by show of hands during meeting: In favour for abstract contract instead of contract abstract.

[chriseth]

To clarify the implementation proposal again:

• cannot be applied to interfaces (for now)
• cannot be applied to libraries (for now)
• abstract is a setting for the contract
• if !isFullyImplemented() (as implemented now) has a different value than abstract(), issue a warning.

[chriseth]

Idea by @axic: An empty contract should be considered not implemented (and thus require abstract).

[nventuro]

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.

That can also be achieved by making the constructor internal:

contract Ownable {
  constructor() internal {
    _owner = msg.sender();
  }

 ...
}

This is IMO a cleaner approach, since it directly tackles the actual objective: preventing the contract from being deployed.

I'd have the abstract keyword cause a compiler error when applied to a contract that has no pure virtual functions. That way, it will always have the same meaning: the contract contains function declarations with no definition.

[chriseth]

Implemented in https://github.com/ethereum/solidity/pull/7358