Functions returning extra returndata unexpectedly revert

[ernestognw]

Component

Forge

Have you ensured that all of these are up to date?

• Foundry
• Foundryup

What version of Foundry are you on?

forge 0.2.0 (05d6062 2024-01-10T00:17:37.314532000Z)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

Summary

In Solidity, when decoding returndata and calldata arguments, extra data is always ignored. This behavior is the same as using abi.decode:

uint256 x = abi.decode(hex"f000baar00000000000000000000000000000000000000000000000000000000", (uint256))
uint256 y = abi.decode(hex"f000baar000000000000000000000000000000000000000000000000000000001234", (uint256))
assert(x != y);

Traces:
  [843] 0xBd770416a3345F91E4B34576cb804a576fa48EB1::run()
    └─ ← panic: assertion failed (0x01)

However, when testing this behavior using Foundry, the transaction reverts instead of just ignoring the extra data.
A simple test in Sepolia confirms that the transaction actually goes through.

Replication

Consider the following two contracts:

// src/ERC1271.sol
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

interface IERC1271 {
    function isValidSignature(bytes32 hash, bytes memory signature) external view returns (bytes4 magicValue);
}

contract ERC1271Wallet {
    struct SignatureFailStruct {
        bytes4 requested_code;
        bytes4 fail_status;
        bytes4 fail_reason;
    }

    function isValidSignature(
        bytes32,
        bytes memory
    ) external pure returns (SignatureFailStruct memory) {
        return SignatureFailStruct(0x1626ba7e, 0x00000000, 0xffffffff);
    }
}

contract VeryCriticalContract {
    uint256 foo;

    function callERC1271isValidSignature(
        address _addr,
        bytes32 _hash,
        bytes calldata _signature
    ) external {
        bytes4 result = IERC1271(_addr).isValidSignature(_hash, _signature);
        require(result == 0x1626ba7e, "INVALID_SIGNATURE");

        foo++;
    }
}

When calling callERC1271isValidSignature(address(wallet),bytes32(0),"") in VeryCriticalContract the transaction doesn't revert when using a testnet, but it does revert when using forge test as the following test confirms:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.22;

import {Test} from "forge-std/Test.sol";
import "../src/ERC1271.sol";

contract ERC1271Test is Test {
    ERC1271Wallet public wallet;
    VeryCriticalContract public someContract;

    function setUp() public {
        wallet = new ERC1271Wallet();
    }

    function testCallERC1271isValidSignature() public {
        vm.expectRevert();
        someContract.callERC1271isValidSignature(
            address(wallet),
            bytes32(0),
            ""
        );
    }
}

Running 1 test for test/ERC1271.t.sol:ERC1271Test
[PASS] testCallERC1271isValidSignature() (gas: 10061)
Traces:
  [10061] ERC1271Test::testCallERC1271isValidSignature()
    ├─ [0] VM::expectRevert(custom error f4844814:)
    │   └─ ← ()
    └─ ← EvmError: Revert

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.60ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Expected behavior

When running forge test, the result should be the same as in the specified Sepolia transaction. Such transaction was also done with forge scripting:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Script.sol";
import "../src/ERC1271.sol";

contract ERC1271Script is Script {
    function setUp() public {}

    function run() public {
        vm.startBroadcast();
        ERC1271Wallet wallet = new ERC1271Wallet();
        VeryCriticalContract someContract = new VeryCriticalContract();
        someContract.callERC1271isValidSignature(
            address(wallet),
            bytes32(0),
            ""
        );
        vm.stopBroadcast();
    }
}

[grandizzy]

the problem here is that you miss creating the VeryCriticalContract in test setup, adding

    function setUp() public {
        wallet = new ERC1271Wallet();
        someContract = new VeryCriticalContract();
    }

yields proper result

[PASS] testCallERC1271isValidSignature() (gas: 34047)
Traces:
  [34047] ERC1271Test::testCallERC1271isValidSignature()
    ├─ [26665] VeryCriticalContract::callERC1271isValidSignature(ERC1271Wallet: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 0x0000000000000000000000000000000000000000000000000000000000000000, 0x)
    │   ├─ [873] ERC1271Wallet::isValidSignature(0x0000000000000000000000000000000000000000000000000000000000000000, 0x) [staticcall]
    │   │   └─ ← [Return] SignatureFailStruct({ requested_code: 0x1626ba7e, fail_status: 0x00000000, fail_reason: 0xffffffff })
    │   └─ ← [Stop] 
    └─ ← [Stop] 

we're looking to improve error reporting when interacting with undeployed contracts in #4141. Closing this, thank you!