blog.md

Imagine you are writing a Solidity smart contract and one of its properties can be characterized like type or state. In other words, something from a limited set of options. You immediately say to yourself "Great, I'll just use enum type for this state variable." On the one hand, this approach has some benefits like increased readability. On the other hand, it can easily take you down a tricky road potentially leading to problems.

What are you talking about?

Well, everything is OK if enum members are encapsulated within only one contract and are never ever mentioned in other files. However, DApps usually consist of several contracts connected with each other. The problem I am talking about appears when the same enum:

1. Appears in more than one contract and
2. Is modified during the DApp lifecycle.

For example, you have 2 contracts. The first one is a kind of storage for very important information. You also declare an interface with the enum definition to refer to it.

contract  IStorage {    
    enum  RecordState {StateA, StateB}
    function  setState(address user, RecordState newState) public;    
}
    
contract  Storage  is IStorage { 
    mapping(address=>RecordState) public states;
    
    constructor() public {}
    
    function  setState(address user, RecordState newState) public {    
	   states[user] = newState;    
    }
}

Each user's record is represented with a enum of 2 possible options: StateA and StateB. setState function is able to change user's state. There is also another contract that end users are supposed to interact with (for simplicity I am omitting access control modifiers in Storage contract).

contract  StorageUser {
    IStorage public recordStorage;
    
    constructor(IStorage _recordStorage) public {	    
	    recordStorage = _recordStorage;
    }
    
    function  changeStateA() external {	    
	    recordStorage.setState(msg.sender, IStorage.RecordState.StateA);	    
    }

    function  changeStateB() external {
	    recordStorage.setState(msg.sender, IStorage.RecordState.StateB);	    
    }        
}

Then you deploy those contracts to the blockchain. Everything is OK: you call either changeStateA or changeStateB and Storage contract's data gets modified accordingly via its own setState function. But one day you realize that you need a completely new state option for some brand new feature. You call it StateC (wow! what a naming!). First, you modify the source code by adding new enum member in IStorage...

enum RecordState {StateA, StateB, StateC}

...and a new method for StorageUser.

function changeStateC() external {
	recordStorage.setState(msg.sender, IStorage.RecordState.StateC);
}

Moreover, as a responsible developer you write tests that call new method and they report success. Your plan was to redeploy only StorageUser contract and you don't want to redeploy Storage, there is a lot of important data in the form of mapping that is quite hard to migrate. So StorageUser is redeployed with the current Storage as its constructor parameter. You call new changeStateC function .. and it fails. Wait, what?! Why? Tests were OK.

Source of the failure

You see, updated StorageUser knows about 3 members of the RecordState enum, but old Storage doesn't have a clue about new StateC option. It can't convert setState function argument StateC to its version of the enum and therefore fails.
What's more, your tests might have fooled you because they used the updated version of both contracts.
Actually, you could even have read the warning regarding this issue in the official docs. The explicit conversion from integer checks at runtime that the value lies inside the range of the enum and causes a failing assert otherwise. The sad truth is sometimes you just overlook such things and bump right into those issues yourself.

Lessons to learn

First, in the case like described above it's way better to replace enums with plain integers. Yes, they don't look so good but the resulted structure is more reliable and expandable. Second, don't throw the whole idea of using enum fields away. If such field lies within one and only one contract, it is absolutely safe. It is also safe if you can ensure that all contracts using the enum are redeployed altogether in case of modification. Remember, the problems appeared when enum first was imported from IStorage to StorageUser contract and only the latter was redeployed after the modification of initial members.
Just don't forget, that if you really want to use enum in your contracts, it's better to think twice.

Try yourself

Source code of this example is uploaded to GitHub. I've reproduced the problematic behaviour in the test file. Feel free to inspect it and experiment yourself.

Credits

1. Image of people in the forest. Photo by Dewa Prabawa from Pexels
2. Image of a kitten. Photo by Vadim B from Pexels