Virtual Keyword Considered Harmful

[ItsNickBarry]

Virtual Keyword Considered Harmful

The paternalistic, oppressive, and anti-intellectual introduction of the virtual keyword to Solidity has, in a single stroke, made Solidity contract development libraries almost useless for non-trivial applications. It behooves library maintainers to declare all functions virtual until, at least, the day that the Solidity compiler responds to the overriding of a non-virtual function with a warning rather than an error.

Is it dangerous to override view functions?

No. @frangio provides an example in #2154 of problems that might result from virtual view functions, bit such issues are easy to avoid in practice.

Here is an example of an ERC20 balanceOf function which accounts for a theoretical earnedInterest value that may change over time. It is gas-efficient, and turns the two calls of a claim/transfer pattern into just one:

function balanceOf (address account) override public view returns (uint) {
  return super(account) + earnedInterest(account);
}

function earnedInterest (address account) public view returns (uint) {
  // TODO: return quantity of tokens earned which has not yet been minted
}

function _beforeTokenTransfer (address from, address to, uint amount) virtual override internal {
  _mint(from, earnedInterest(from));

  // TODO: clear earned interest to prevent double minting

  super(from, to, amount);
}

Any functions which truly rely on strict, known behavior of view functions may be able to bypass the problem entirely by specifying the contract name explicitly before a function call:

function strictBalanceOf (address account) public view returns (uint) {
  return ERC20.balanceOf(account);
}

EIP1167 Minimal Proxies

Several OpenZeppelin contracts are currently completely incompatible with the EIP1167 Minimal Proxy pattern. ERC20, ERC721, and EER1155 all rely on constructors to set their metadata, but proxy constructors are never called. Previously, it would have been trivial to override these metadata view functions to point to storage variables accessible to an initialize function, but this is now impossible because such functions are not virtual.

Admittedly, this problem could also be solved by providing access to the storage variables declared in OpenZeppelin base contracts.

/ˈtɛstɪŋ/

The primary argument against end-user control over which functions may be overridden seems to be that OpenZeppelin contracts would not work as expected. But who among us is here to deploy the OpenZeppelin ERC20 reference implementation? The object of relying on a third-party library is to expand upon it, to make innovations unanticipated by its authors. And each of those innovations should be rigorously validated through comprehensive automated testing.

Copy and Paste?

Indeed, one can simply copy and paste the OpenZeppelin contracts, whatever state they're in at the most recent commit. Those who are too clever for OpenZeppelin may resort to this (whether @KaiRo-at is indeed too clever for OpenZeppelin remains an OpenQuestion).

That's how Y2K happened. This time, it will be worse. It will be Y3K.

---

#Y3K
#WebY3K
#ClosedZeppelin
#VirtuallyBreakingChanges

[abcoathup]

Hi @ItsNickBarry,

In case you weren't aware, OpenZeppelin Contracts Ethereum Package contains the same contracts as the vanilla OpenZeppelin Contracts, but modified for use with upgradeable contracts, so have initializer functions rather than constructors.

As for the discussion on making view functions virtual, I recommend contributing use cases to the comments of #2154 to aid the discussion.

I am closing this issue as a duplicate of #2154