Solidity: Stack too deep with external + callcode, compiles fine with public and memory

[androolloyd]

Description

While porting our contracts from solidity 0.4.x -> 0.5.x we ran into a stack too deep issue around external functions

External functions give access to the callcode data directly

Public functions copy variables into memory and then execute them there

It appears that the external functions have less stack space than a regular public function

Environment

• Truffle: v5.0.1 (core: 5.0.1)
• Solidity: v0.5.0 (solc-js)
• Node: v11.0.0
• EVM execution environment / backend / blockchain client: ganache
• Operating system: OSX

Steps to Reproduce

Below the code will build correctly, uncomment the notWorking function and compile to see it fail.

```pragma solidity ^0.5.0;

contract Enum {
enum Operation {
ONE,
TWO,
THREE
}
}

contract Debug {

bytes32 public constant CONTRACT_CONST_ONE = 0x180e6fe977456676413d21594ff72b84df056409812ba2e51d28187117f143c2;
bytes32 public domainSeparator;

function encodeData(
    address one,
    uint256 two,
    bytes memory three,
    Enum.Operation four,
    uint256 five,
    uint256 six,
    uint256 seven,
    address eight,
    address nine,
    bytes memory ten
)
    public
    view
    returns (bytes memory)
{
    bytes32 eleven = keccak256(
        abi.encode(CONTRACT_CONST_ONE, one, two, keccak256(three), four, five, six, seven, eight, nine, keccak256(ten))
    );
    return abi.encodePacked(byte(0x19), byte(0x01), domainSeparator, eleven);
}

function working(
    address one,
    uint256 two,
    bytes memory three,
    Enum.Operation four,
    uint256 five,
    uint256 six,
    uint256 seven,
    address eight,
    address payable nine,
    bytes memory ten,
    bytes memory eleven
)
    public
    returns (bool fourteen)
{
    uint256 twelve = gasleft();

    bytes memory thirteen = encodeData(
        one, two, three, four,
        five, six, seven, eight, nine,
        ten
    );
}

// function notWorking(
// address one,
// uint256 two,
// bytes calldata three,
// Enum.Operation four,
// uint256 five,
// uint256 six,
// uint256 seven,
// address eight,
// address payable nine,
// bytes calldata ten,
// bytes calldata eleven
// )
// external
// returns (bool fourteen)
// {
// uint256 twelve = gasleft();
//
// bytes memory thirteen = encodeData(
// one, two, three, four,
// five, six, seven, eight, nine,
// ten
// );
// }

}

[chriseth]

This is indeed the case. bytes types in calldata need two stack slots, while they only need a single stack slot in memory. The main reason for that is that msg.data is of type bytes calldata and it needs an explicit length on stack, even though this might not be needed for other bytes calldata types.

Could you clarify the scope of this issue (are you reporting a bug, requesting a feature, ...)?

[chriseth]

Since you mention porting from 0.4.0 to 0.5.0: There is absolutely no need to mark the function external just because of 0.5.0. Interface functions have to be external, but implementing functions can be public.

[androolloyd]

Hi Chris the intention was to report a bug, but if this is indeed working as intended, some accompanying documentation around this would be great, most of the peers I checked with led me to believe everything should work exactly as before.

Was hoping to take advantage of the gas reduction provided with external functions.

[PhABC]

I encountered this issue as well, where the following function needs to be set to public in 0.5 instead of external in 0.4 : https://github.com/horizon-games/multi-token-standard/blob/f5100ef10ff58837f23721a18582f6233d1cfaed/contracts/token/ERC1155.sol#L98

Would there be anyway to distinguish msg.data from other types of bytes array? I can imagine other people would prefer setting their functions to external instead of public without having to refactor them.

[chriseth]

calldata arrays have never been fully implemented, but we might change that soon. With the completion of that feature, we might also split the type into two, but that could be quite difficult.

[chriseth]

Closing this.