Not upgrading if bytecode and creation code have not changed

[KholdStare]

Hey!

I was wondering if there is a way to prevent re-deployment of code that basically has not changed (e.g. reformatting, adding comments, or changing function order). Currently those can cause false-positive storage incompatibilities with isUpgradeSafe. However, implementation address is the thing that is actually checked for a difference to know whether something is changed or not. There is no control over this, and a contract will get redeployed in this case.

Is there a way to see if implementation bytecode is identical (even if solidity code has slight cosmetic changes), and opt out of upgrading a contract in that case? I think solidity outputs different bytecode if the source changes (a section towards the end of the bytecode), I'm assuming for verification purposes. Any way to check only the implementation bytecode and no anything that depends on the source?

[0xPhaze]

The thing is that in that case, your bytecode will have changed, that's why it's re-deploying. As a quick fix, you can add the parameter keepExisting = true to setUpContract / setUpProxy and manually set it to false once you want to explicitly have new deployments (or use the global var UPGRADE_SCRIPTS_KEEP_EXISTING=true. Also, be sure to compile the contracts without a metadata hash (foundry.toml setting: bytecode_hash = "none") which may change the bytecode due to comments being altered. Checking for functional equality of the code would be too hard to do.

And I would love to make the script detecting storage layout changes smarter by parsing the ast output or performing some other more in-depth storage analysis, but for the moment I don't feel knowledgable enough to catch odd edge cases. That's why it's better to err on the cautious side, but I might look into that more.

[KholdStare]

Thanks for pointing out the bytecode_hash = "none" option. Unfortunately we don't want to turn that off so that we can verify.

Here is my intuition - Currently because bytecode_hash = "ipfs" for verification purposes, the "overall" bytecode changes even when there are cosmetic changes. However let's say that the "implementation" portion of the bytecode, what is essentially generated when bytecode_hash = "none", remains unchanged. In these cases, it doesn't make sense to redeploy the contract.

I wonder if it would be possible in pseudocode:

function needsUpgrade() returns (bool) {
  if (oldImplementationAddress == newImplementationAddress) return false;

  console.log("Need upgrade");

  if (oldBytecodeWithoutHash == newBytecodeWithoutHash && UPGRADE_SCRIPTS_SKIP_METADATA_CHANGE) {
    console.log("Skipping upgrade because only metadata changed");
    return false;
  }

  return true;
}

Not sure if it's easily possible to get the bytecode of a contract without the hash inside upgrade scripts themselves.

Does it make sense?

[0xPhaze]

That does make sense. https://docs.soliditylang.org/en/latest/metadata.html#encoding-of-the-metadata-hash-in-the-bytecode I could match for that part and exclude it.