A Case Against Inheritance in Smart Contracts

Photo by Matteo Grando on Unsplash

In this article, I’ll attempt to persuade you to reduce your use of inheritance in smart contracts and to increase your skepticism when you see it.

The position that inheritance is to be avoided is, at least to some extent, one of personal preference. I won’t attempt to prove to you that inheritance is bad, but I will show you two examples that will hopefully nudge you in that direction.

Clarity is paramount

Programs must be written for people to read, and only incidentally for machines to execute.
– Harold Abelson, Structure and Interpretation of Computer Programs

This quote from 1984 long predates the blockchain, but it’s especially true when it comes to smart contracts. Smart contracts make it possible for people to engage in all sorts of transactions without having to trust one another. Trust between participants is replaced by trust that the smart contract does what it’s supposed to.

In Upgradeability Is a Bug, I argued that it’s impossible to trust a mutable smart contract because what it does may change. Similarly, it’s impossible to trust an inscrutable smart contract because what it does is too hard to determine in the first place.

Inheritance is one way to make smart contracts harder to understand. This is a problem for people who read the code and for the developers who need to maintain it.

Inheritance is hard for readers

This short example relies heavily on inheritance. See if you can spot the vulnerability:

After calling disableAdmin, no one should be able to call kill successfully, but it turns out they can. This vulnerability could be an innocent mistake, but it’s more likely intentional deception by the author, who can now run off with everyone’s ether.

The problem relates to multiple inheritance. Bank inherits from both TempAdmin and MultiAdmin. When the kill function calls isAdmin, which contract’s version of that function is executed? When that contract subsequently calls super.isAdmin, what happens?

The Solidity compiler uses an algorithm called C3 linearization to answer these questions. In this specific example, the code is executed in this order:

kill → MultiAdmin.isAdmin → TempAdmin.isAdmin → Admin.isAdmin

This is a bad order, leading to checking the equivalent of this:

What we would prefer, and what happens if you write is MultiAdmin, TempAdmin (swapping the two base classes), is this order:

kill → TempAdmin.isAdmin → MultiAdmin.isAdmin → Admin.isAdmin

This results in correct code that checks the equivalent of this:

This example was heavily inspired by Phil Daian’s entry in the 1st Underhanded Solidity Coding Contest. If you’d like a deeper understanding of the issue and a more devious smart contract example, I encourage you to read his writeup and code in full.

Inheritance is hard for developers

Consider the following contract, which implements a marketplace for ERC721 non-fungible tokens:

This marketplace is perfectly functional, but perhaps the NFT market is volatile, and users would like the ability to set expiration dates on their listings. The following contract inherits from NFTMarket and adds expiration dates:

I couldn’t modify the original contract’s Listing struct, so I’ve added a new array to keep track of expirations. I had to override all the functions, and readers have to jump back and forth between the base class and the derived class to understand what’s happening.

Did you spot the bug? I completely missed it in the first draft of this article, and inheritance is all I was thinking about! Because my new list function takes an extra parameter, it’s not actually overriding the original. I need this too:

Without this override, callers will still be able to invoke the base contract’s list function, and the listing array will get out of sync with the expirations array. (Exactly what would happen is left as an exercise to the reader, but suffice it to say that the contract would be thoroughly broken.)

The derived contract is hard to read and hard to maintain. Depending on how you format the code, the new contract increases our total line count by about 75%.

Copy/paste/modify is better

Now I’ll take a different approach. Instead of inheriting from NFTMarket, I’ll copy/paste the code and directly modify it. In the below code, I’ve annotated every change I made with a comment that reads “ADDED”:

This time, it took only four lines of code to add the needed functionality! The result is much easier to read and was trivial to get right on the first try. When I add new functions in the future, it will be straightforward to take expirations into account.

Should you use inheritance at all?

Vyper, an Ethereum smart contract language that aims to fix some of Solidity’s shortcomings, does away with inheritance altogether:

Class inheritance — requires people to jump between multiple files to understand what a program is doing, and requires people to understand the rules of precedence in case of conflicts (which class’ function X is the one that’s actually used?). Hence, it makes the code too complicated to understand.

I’d love to tell everyone to just never use inheritance, but here are a few arguments in favor of it:

1. Using inheritance in conjunction with private functions can actually simplify things. A derived class can’t call its parents’ private functions directly, so there’s a little less surface area to consider.
2. Inheritance may make it easier to maintain multiple versions of a contract that support different use cases. OpenZeppelin is an example of a library that uses inheritance in this way. Arguably, inheritance makes these contracts harder to read, but it makes it easier for the developers to maintain lots of variants.
3. If you want to reuse someone else’s code, inheritance may be the cleanest way to do that. Deriving from a contract, especially a popular one like OpenZeppelin’s Ownable contract, may actually make it easier for a reader to understand your code. This is especially true if you don’t override any of the base contract’s functions. Copy/pasting would make it harder for readers to determine whether you made any modifications.

Bottom line

I encourage you to think of inheritance like inline assembly: there are situations that call for its use, but it should be avoided when possible because it hurts readability and maintainability.