Fix non-matching metadata hash in solidity compilation

[sveitser]

The solidity compiler appends an IPFS hash of the metadata and the compiler version at the end of the bytecode.

https://docs.soliditylang.org/en/latest/metadata.html#encoding-of-the-metadata-hash-in-the-bytecode

At the moment despite the metadata being the same (can be checked by running forge build --extra-output-files metadata --force and finding the metadata files in contracts/out) the IPFS hash is sometimes different. This is very troublesome because we want to generate consistent bindings on all our dev machines and the CI.

It may be possible to disable this metadata hash but that's not desirable. Currently waiting for foundry community to see if they have any suggestions about how to further debug this.

Another maybe related problem is differences in bindings (not just due to bytecode differences) in the CI vs locally.

cc @alxiong

[sveitser]

@alxiong For the erc20 issue. I'm wondering if it could have something to do with having multiple erc20 tokens in the repo.

[sveitser]

I'm 90% sure the issue with the ERC20 is that we use two different ERC20 token implementations, one from solmate and one from openzeppelin. If we're lucky this will also fix the bytecode issue.

[alxiong]

so what's happening under the hood that cause the discrepancy in bytecode? (is this a Mac v.s. Linux thing or a Forge bug or? Why would two ERC20 impl cause this, afterall the bytecode should be deterministically derived from our contract code which is unambiguous about which ERC20 we used, correct?)

[sveitser]

Not sure. I think the two erc20 implemenations don't cause the bytecode_hash issue, but the issue with mismatches in the erc20 bindings.

My guess would be that one of the contracts is chosen but the way that happens is not deterministic. I would consider that a foundry bug.

Regarding the bytecode mismatch it could be due to a great number of things. I was thinking maybe some recent changes but I now also have this issue on main where I locally don't generate the same bindings that we have checked in. I don't currently have the bandwidth to look more into this and think disabling the hash is fine, but we can keep this issue open.

[sveitser]

The metadata also includes dependencies and remappings so any changes to those (like different remappings) should also make the bytecode hash change.

   ...
    "remappings": [
      ":@openzeppelin/contracts/=contracts/lib/openzeppelin-contracts/contracts/",
      ":bn254/=contracts/lib/bn254/src/",
      ":ds-test/=contracts/lib/solmate/lib/ds-test/src/",
      ":erc4626-tests/=contracts/lib/openzeppelin-contracts/lib/erc4626-tests/",
      ":forge-std/=contracts/lib/forge-std/src/",
      ":openzeppelin-contracts/=contracts/lib/openzeppelin-contracts/",
      ":solidity-bytes-utils/=contracts/lib/solidity-bytes-utils/contracts/",
      ":solmate/=contracts/lib/solmate/src/"
    ]
  },
  "sources": {
    "contracts/lib/bn254/src/BN254.sol": {
      "keccak256": "0x3a1a56e5130ce711cf9c9d63a28dd9025cfc02a8676836122fa036403539aee1",
      "license": "GPL-3.0-or-later",
      "urls": [
        "bzz-raw://d6f9865d01cb2ad3163f3e3f59357e712d15a5544c93e08f35d9074f08638dcb",
        "dweb:/ipfs/QmYwZE6G1w8KD8MMv31ZLj3AeDJSPLhFGTXiPRGciF9eDj"
      ]
    },
    "contracts/lib/bn254/src/Utils.sol": {
      "keccak256": "0x9d7fec75027e3c8a652a07f003942ca57a5d614fc6f8bb6f7982ab3d258902fc",
      "license": "GPL-3.0-or-later",
      "urls": [
        "bzz-raw://56a4f6a824b5fca02386fe90887c5ca7771dfdad7a5546c6518179afd7344a10",
        "dweb:/ipfs/QmV4SWwDrwqh3WEkg2uQpZe58AjR8Cc2F9tPYYcwc7HKDq"
      ]
    },
    "contracts/lib/solidity-bytes-utils/contracts/BytesLib.sol": {
      "keccak256": "0xf75784dfc94ea43668eb195d5690a1dde1b6eda62017e73a3899721583821d29",
      "license": "Unlicense",
      "urls": [
        "bzz-raw://ca16cef8b94f3ac75d376489a668618f6c4595a906b939d674a883f4bf426014",
        "dweb:/ipfs/QmceGU7qhyFLSejaj6i4dEtMzXDCSF3aYDtW1UeKjXQaRn"
      ]
    },
    "contracts/src/interfaces/IPlonkVerifier.sol": {
      "keccak256": "0x9ffd322ff7533729370d205d8436b9b5bb3f393e3f58bff2e8917221dd04d8be",
      "license": "Unlicensed",
      "urls": [
        "bzz-raw://159cd2080a79d0f1ce60c829692f8900646fd1dfea198b4878d57b896961088b",
        "dweb:/ipfs/QmV9oJsGzbzMR1Ufst3P82X1pnaExsLhpidJUEHA4mPWHL"
      ]
    },
    "contracts/src/libraries/PlonkVerifier.sol": {
      "keccak256": "0x4bf2c2da0027bde8ffc4a57da70cd62d55654e75506c5612c927d2b8147c94c0",
      "license": "Unlicensed",
      "urls": [
        "bzz-raw://433425b7222a99198e644698ef0ce3e0563f60bd97cc5e33bdceeffe4c814dcf",
        "dweb:/ipfs/QmeAK66LU4aLyjwrwHF2RoChxnjGAimHeu6dqW95ZKrdZA"
      ]
    },
    "contracts/src/libraries/PolynomialEval.sol": {
      "keccak256": "0xd908aece9ccbd0dfe09d775c0cf2e4bdb8064532305e29e452734afb611b10f3",
      "license": "Unlicensed",
      "urls": [
        "bzz-raw://7f01f93d6539f3508eb3cf6c261f828514f4659bfc5ad0cebb7407cf5ac5bc31",
        "dweb:/ipfs/QmPTAjtWqfmTSY2pcVs9nWerSKMQarFVVp9HgMXHTJkAyz"
      ]
    },
    "contracts/src/libraries/Transcript.sol": {
      "keccak256": "0xee7dfe1bb61dadcc07438edde77d54b0025359321918a4d7568e24e645e9fe66",
      "license": "Unlicensed",
      "urls": [
        "bzz-raw://b07b77c0d38000068409f6ed143348240f0fdf7cf3a7fcba7a3184e079fd7f44",
        "dweb:/ipfs/QmVXQN6qBkby35aiRNutKvSQ2Ybq5PA5JnuVutUf2RpfKh"
      ]
    }
  },

Full file: PlonkVerifier.metadata.json

[alxiong]

should we consider this issue as fixed, now that fix in commit is merged into main?

[sveitser]

I would prefer if we had deterministic bytecode hashes again and didn't need to disable it in

https://github.com/EspressoSystems/espresso-sequencer/blob/58852cbd989606f103a41664ef4c728859de007d/foundry.toml#L19