argotorg / argotorg/solidity

Don't require `override(A, B)` when inherited contracts already inherit from each other

Open
#12,554 7 comments 4 reactions 0 assignees View on GitHub
language design :rage4: medium effort medium impact must have needs design
Dominant language
C++
Stars
25.7k
Forks
6.2k
Avg merge
1d 11h
Merged PRs (30d)
21

Description

## Description
Let's consider a situation where we have a contract `C` that inherits from `A` and `B` where `B` already inherits from `A`. `A` provides a function that `B` overrides:

```solidity
contract A {
function foo() public virtual {}
}
contract B is A {
function foo() public virtual override {}
}
contract C is A, B {}
```
Currently the compiler reports an error if `C` does not define `function foo() public virtual override(A, B)`:
```
Error: Derived contract must override function "foo". Two or more base classes define function with same name and parameter types.
--> test.sol:7:1:
|
7 | contract C is A, B {}
| ^^^^^^^^^^^^^^^^^^^^^
Note: Definition in "A":
--> test.sol:2:5:
|
2 | function foo() public virtual {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Note: Definition in "B":
--> test.sol:5:5:
|
5 | function foo() public virtual override {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

Technically, the compiler is right because both `A` and `B` provide conflicting definitions for `foo()` (the original one and an overridden one). In practice though, due to how multiple inheritance works in Solidity, the user will expect the explicitly inherited `A` and the one inside `B` to be the same thing so it would make sense to spare the user the error and just choose `B.foo()` automatically.

## Context
This originally came up in this thread on OZ forum: [// The following functions are overrides required by Solidity](https://forum.openzeppelin.com/t/the-following-functions-are-overrides-required-by-solidity).

While the case I'm showing above may look like an obscure corner case, such code is actually something users encounter often because [OpenZeppelin's Contract Wizard](https://docs.openzeppelin.com/contracts/4.x/wizard) generates code where contracts inherit from other contracts that are already inherited by one of base contracts. I have also often seen this pattern in real-life contracts - users often inherit `ERC20` explicitly even if one of the other contracts they inherit already does.

Here's an example you get from the wizard when you just select the `Votes` checkbox:
```solidity
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.2;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/extensions/draft-ERC20Permit.sol";
import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Votes.sol";

contract MyToken is ERC20, ERC20Permit, ERC20Votes {
constructor() ERC20("MyToken", "MTK") ERC20Permit("MyToken") {}

// The following functions are overrides required by Solidity.

function _afterTokenTransfer(address from, address to, uint256 amount)
internal
override(ERC20, ERC20Votes)
{
super._afterTokenTransfer(from, to, amount);
}

function _mint(address to, uint256 amount)
internal
override(ERC20, ERC20Votes)
{
super._mint(to, amount);
}

function _burn(address account, uint256 amount)
internal
override(ERC20, ERC20Votes)
{
super._burn(account, amount);
}
}
```

The only reason for including the overridden functions is the problem described in this issue. Without it the contract would have been as short as
```solidity
contract MyToken is ERC20, ERC20Permit, ERC20Votes {
constructor() ERC20("MyToken", "MTK") ERC20Permit("MyToken") {}
}
```

The same effect could be achieved by removing `ERC20` and `ERC20Permit` from the inheritance list but, like I already mentioned, that's not something people do in practice.

Contributor guide

Open the contributing guide

Research direction

Start by reproducing the Solidity examples with contracts A, B, and C, then the OpenZeppelin ERC20/ERC20Votes inheritance example. Trace the compiler's multiple-inheritance override validation and determine whether C can select B.foo() automatically. Done means the examples compile without explicit redundant overrides while preserving correct override semantics and diagnostics for genuine conflicts.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, solidity
Domain
blockchain, compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.