Add "unchecked" modifier to disable msg.data length checks

[Agusx1211]

Abstract

Solidity 5 introduces a new sanity-check on the msg.data of a function if the msg.data length differs from the defined by the signature the transaction fails. Some mainstream contract implementations perform custom calls without taking into consideration this rule, those implementations are not able to interact with newly compiled Solidity 5 contracts.

Ej: The reference implementation for introspection of an ERC165 compliant contract sends the wrong msg.data size ethereum/EIPs#1640

Motivation

Adding an unchecked function modifier would disable the check on the msg.data bounds, allowing broken contract callers (as ERC165) to interact with these new contracts.

interface ERC165 {
    function supportsInterface(bytes4 interfaceID) unchecked external view returns (bool);
}

Specification

Any function containing the unchecked should ignore any validation on the msg.data size, functions without unchecked should behave as described on the Solidity 5 changelog

Code Generator: Revert at runtime if calldata is too short or points out of bounds. This is done inside the ABI decoder and therefore also applies to abi.decode().

[chriseth]

I don't think this is a good idea. We have to progress instead of keeping unsafe backwards-compatible behaviour. Note that this change was even communicated to the authors of this particular example code ( ethereum/EIPs#881 (comment) ). The problem was that it had a bug that was already present before the change that introduced the padding. I know that I myself did not spot the bug, but that also means that the process has to be improved.

[Agusx1211]

This may not be the best way to address this kind of issue, but in the last version of Solidity there is no way to implement a retro-compatible ERC165, maybe a different solution should be discussed.

I don't agree with the idea of "avoiding unsafe backwards-compatible behavior", most of the contracts are immutable, and Solidity is a contract-oriented programming language, it should have legacy compatibility between the top priorities.

About the ERC165 bug, the process should be improved, but no process is flawless, and with more and more complex "standards", this is bound to happen again.

[chriseth]

A contract should either be ready for changes in its environment or it should be short-lived. It should certainly be ready to contain bugs and not expect other contracts to change to adapt to its faulty behaviour.

[spalladino]

@Agusx1211 as discussed offline, I understand that having a global modifier that allows you to inject code before the function dispatch should be able to solve this. WDYT? See #3864.

[Agusx1211]

@spalladino Totally, #3864 looks like a good candidate to solve almost any compatibility issue with newer versions of Solidity.

[Agusx1211]

@chriseth That's not the case with all the ERC20 contracts, there is a good portion with breaking bugs, nonetheless achieve consensus to migrate to a new contract is hard.

The way I see it, a programming language oriented to Smart contracts should have a primary focus on Security and Legacy compatibility

[chriseth]

@Agusx1211 isn't the problem that security and legacy compatibility are often conflicting?