Different bytecode produced via IR for some of OpenZeppelin's contracts when extra contracts are included in the input

cameel commented 1 month ago

Description

This looks like yet another bug where different AST IDs lead to differences in the generated bytecode. I found it when experimenting with parallel compilation of OpenZeppelin.

When each source file is compiled in isolation by providing only that single file as input (rather than providing all of them and only selecting one via outputSelection), three contracts produce different bytecode than when all contracts are compiled together:

test/metatx/ERC2771Forwarder.t.sol:ERC2771ForwarderTest
test/token/ERC20/extensions/ERC4626.t.sol:ERC4626StdTest
test/utils/structs/Checkpoints.t.sol:CheckpointsTrace224Test

Environment

Compiler version: 0.8.26
Target EVM version (as per compiler settings): cancun

Steps to Reproduce

git clone --depth=1 https://github.com/OpenZeppelin/openzeppelin-contracts --branch v5.0.2
cd openzeppelin-contracts/
forge install

cat <<EOF > input.json
{
    "language": "Solidity",
    "sources": {
        "contracts/mocks/token/ERC4626Mock.sol":     {"urls": ["contracts/mocks/token/ERC4626Mock.sol"]},
        "test/token/ERC20/extensions/ERC4626.t.sol": {"urls": ["test/token/ERC20/extensions/ERC4626.t.sol"]}
    },
    "settings": {
        "remappings": [
            "@openzeppelin/contracts/=contracts/",
            "ds-test/=lib/forge-std/lib/ds-test/src/",
            "erc4626-tests/=lib/erc4626-tests/",
            "forge-std/=lib/forge-std/src/"
        ],
        "metadata": {"appendCBOR": false},
        "outputSelection": {"*": {"*": ["evm.bytecode"]}},
        "viaIR": true
    }
}
EOF

cat input.json \
    | solc --standard-json > output-select2.json
cat input.json \
    | jq 'del(.sources."contracts/mocks/token/ERC4626Mock.sol")' \
    | solc --standard-json > output-select1.json

diff --color --unified \
    <(cat output-select2.json \
        | jq '.contracts."test/token/ERC20/extensions/ERC4626.t.sol"."ERC4626StdTest".evm.bytecode.object' \
        | fold --width 160 \
    ) \
    <(cat output-select1.json \
        | jq '.contracts."test/token/ERC20/extensions/ERC4626.t.sol"."ERC4626StdTest".evm.bytecode.object' \
        | fold --width 160 \
    )

Note: This is a minimized example that's enough to reproduce one of the bytecode differences. With the full input there are more of them and they may or may not have the same cause. To make sure, when the bug is fixed, verify with the full original input: openzeppelin-5.0.2-full-input.json. It should produce the same bytecode for the three contracts when compiled as is and when you include only the source files containing those contracts.

hedgar2017 commented 1 month ago

Also reported to me as a presumably ZKsync toolchain bug with the EAS project: https://github.com/zkSync-Community-Hub/zksync-developers/discussions/507

Please let me know if you need any help with reproducing.

cameel commented 3 weeks ago

@hedgar2017 Is it the same bug though? Unless it's happening in code they import from OpenZeppelin, it may very well be a distinct bug producing similar effects. We've had quite a few bugs of this kind and we're still finding new ones. We've been fixing them one by one, but they're really hard to avoid. Which is why @clonker is currently reworking the way optimizer handles names to eliminate any possibility of AST IDs influencing its operation.

If you can reproduce that error, it would be useful for confirming that we actually fixed it properly when we're done with it. Also in case the change we're preparing does not work out for some reason, it would allow us to still address this case individually. Hopefully we won't have to though and the fix will address all of them generically.

ethereum / solidity