Add support for `calldata` input and output in `abi.decode()`

[k06a]

Abstract

Currently abi.decode() works with memory argument and not with calldata, if calldata is provided it is being copied to memory and then decoded. Let's make abi.decode() to support both arguments and returns as calldata.

Motivation

Calldata abstraction is cool, slicing is cool. We need abi.decode() to fully support decoding from calldata to calldata!

Specification

function f(bytes calldata data) external {
    (
        address[] calldata targets,
        bytes[] calldata calldatas
    ) = abi.decode(data[1:], (address[], bytes[]));

    // ...
}

Backwards Compatibility

[0xmikko]

It could also add additional security because calldata is immutable, so no way to change data where it's not needed.

[ekpyron]

Just to clarify here: decoding from calldata (including slices) does work already (if it isn't for any case in which it should, let us know).
Encoding to calldata types is an interesting idea! So the expectation here would be that the abi.decode call validates the outermost tuple (reverting on invalid data) and returns the appropriate calldata references, basically as if they had been function arguments (only relative to the given bytes calldata instead of msg.data[4:])?

[k06a]

@ekpyron exactly. I assume implementing this feature would require reusing existing code of arguments decoding.

[ekpyron]

Currently, the return type of any function call is fully determined by the types of its arguments - so that'd be quite a step to break here (i.e. actually determine that the full abi.decode(...) expression is assigned to calldata types instead of memory types, and changing the semantics of the expression based on that).

However, I wonder if there are any issues with actually always having abi.decode return calldata types (if given a first argument in calldata), since they should be assignable to memory types as well (which would then trigger the full copy) - if that works, that might be worthwhile. The main issue then would be that it might still change the revert semantics of a plain abi.decode that doesn't use the return arguments...

[ekpyron]

So in general, the easiest solution to this would be to just change the return types of abi.decode with a calldata argument, to calldata types - that should directly solve this with near-zero effort - and I think this should all work now (there is a comment in the type checker code for abi.decode about the implementation of calldata types lacking and hence memory types being forced, but I think that's utterly outdated by now).

The problem with that approach, though, is that it's technically a breaking change :-).

[k06a]

@ekpyron I believe that we need to have 2 functions:

• abi.decode(bytes memory, *)
• abi.decode(bytes calldata, *)

As you said calldata argument version will return calldata types, which can be deep-copied to memory types. It doesn't look like a breaking change for me.

[ekpyron]

abi.decode(bytes calldata, *) already exists, though, but so far is just an alias and returns memory types (and even if it didn't exist, since calldata to memory conversions are implicit effectively current code works as if it existed). And changing those return types to calldata types will change the revert semantics in certain corner cases. I'm saying it's technically breaking, since it's only weird cases that would actually change in behaviour, but strictly speaking it is.

So concretely:

contract C {
  function f() external {
    abi.decode(msg.data[4:], (address[]));
  }
  function test() public returns (bool) {
    uint[] memory x = new uint[](1);
    x[0] = type(uint256).max;
    (bool y,) = address(this).call(abi.encodeWithSelector(this.f.selector, x));
    return y;
  }
}

Calling test() here will currently return false, since abi.decode, despite having a calldata argument, will directly copy-decode to memory, which validates that the address elements don't have dirty higher order bits (but the single array element passed in has all bits set).

However, if we change the abi.decode(bytes calldata, *) version to returning calldata types, the abi.decode-expression in f itself won't perform this validation just yet, so test() will return true.

So yeah - it's the same valid code behaving differently due to changing this, making this at least technically breaking (even though it's a rather annoying technicality here...).

[k06a]

@ekpyron interesting, but why would you check high bits of addresses? I would say it is both unexpected from developer POV and non-desired, since it will spend gas, while function arguments decoder I assume skip these checks.

[ekpyron]

Calling function f(address x) external with dirty higher-order bits in calldata will also revert during argument decoding... Calling function f(address[] callata x) external will not immediately revert if called with an element with dirty higher order bits, but the first use e.g. x[0] will revert, if there are dirty higher-order bits.
Validation like that has been happening ever since ABIEncoderV2.
There's some corner cases when copying from calldata to memory in legacy code generation that won't perform this validation, which is actually underdocumented currently, but in general, there will be input validation.

[ekpyron]

Compared to masking away, the increased gas costs in validating the arguments is not that high - and either of those you need anyways, since just allowing invalid inputs may lead to all sorts of unexpected problems. And validating instead of just masking is an additional safeguard against unexpected behaviour (after all, there's little valid reason for passing garbage in higher-order bits, so if there are, you can usually assume something went wrong and reverting is the safer cause of action).

[k06a]

@ekpyron interesting, didn't noticed that behaviour perviously. So regarding the abi.decode(calldata) this issue could be handled during copying calldata to memory.

[ekpyron]

If you actually have

address[] memory x = abi.decode(msg.data[4:], (address[]));

you'd get a calldata to memory copy, which could actually do the validation.

However, there's nothing preventing you currently from just calling abi.decode itself without using its return values. There's little reason for ever doing so, but that case (so the snippet in #13518 (comment)) would still technically break...
It's reasonable to consider whether this breaking pattern is that weird, s.t. it's fine to break it in a non-breaking release, though.

But yeah, thinking through this made me actually find some inconsistencies in the validation behavior of such calldata-to-memory copies (arrays with statically-sized base types in legacy code generation still use some old pre-ABIEncoderV2-code-paths that do not do this validation, while via-IR code generation always does, and legacy code generation for arrays with dynamic base types also always does), so we'll first need to decide how to deal with those.