Automatic downgrade of default EVM version may not be welcome

[SKYBITDev3]

I think the reversion of default EVM version of the Solidity compiler back to paris (from shanghai, currently the latest) warranted a minor (instead of patch) version number increment (e.g. 2.18.x), as the behavior and outputs have been changed.

People who have been developing without issue based on what was default may only find out by surprise after updating and noticing that all of their contracts got recompiled. Others may not even notice it and proceed with deployments. Such an automatic downgrade of EVM version and recompilation may not be welcome.

A version update from 2.17.2 to 2.17.3 appears on the surface to be a bug fix, so most people may not check the change log whenever there are such small patch version number increments.

Also, people who newly install hardhat may want and expect to only use the latest versions of everything and would only accept a downgrade if they must. So such a downgrade would not be welcome for those who use Ethereum or Polygon zkEVM, as the PUSH0 EVM opcode works fine on those.

Maybe there should be a new message about this when users actually do a compile so that they can take action beforehand instead of letting hardhat do an automatic downgrade almost invisibly.

One idea may be to no longer have evmVersion optional, so that an error appears, forcing the user to choose which one they really want to use and to add it explicity into hardhat.config.js.

Also, it may be good to print the actual solc version and EVM version that is being used when compiling. I show the hardhat solidity settings in my own hardhat-yul repository, e.g.: https://github.com/SKYBITDev3/hardhat-yul/blob/9f842908faef020ed37e36b27edb30a3a5c52e44/src/compilation.ts#L113

[fvictorio]

Hi @SKYBITDev3. I think I agree with you. I considered doing a minor and for some reason then I didn't. In retrospect that was a mistake. But I think undoing that change in a new patch an releasing a new minor with the proper change is going to be too noisy now.

I think the best we can do now is this:

Also, it may be good to print the actual solc version and EVM version that is being used when compiling.

I really like this idea.

With respect to making the evmVersion mandatory... I think that might be too much.

Also, people who newly install hardhat may want and expect to only use the latest versions of everything and would only accept a downgrade if they must. So such a downgrade would not be welcome for those who use Ethereum or Polygon zkEVM, as the PUSH0 EVM opcode works fine on those.

This makes sense, but we do need to make a decision one way or another: we either unnecessarily downgrade the EVM version for some users, or we default to compiling broken bytecode for others. All things considered, I think defaulting to the safer version makes more sense. The cost is that some users will have contracts that could be more optimized but aren't, which seems like a sensible trade-off to me. Wdyt?

[SKYBITDev3]

Best practice generally in technology is to use the latest versions. We resort to a downgrade only if necessary, e.g. if something that we need doesn't work.

Us developers aren't mindless non-technical end users - when something doesn't work, we ourselves find a way to make it work using our problem-solving skills. In the case of EVM version, if we came across invalid opcode error, we would do some searching and find that the solution is simply to downgrade by adding evmVersion: 'paris' in hardhat.config.js. There are around 5 different invalid opcode questions on Stack Overflow with that solution. But if things were fine for us with the latest versions, nothing needs to be done, so you shouldn't be changing our environment without our consent.

A smarter solution, if possible, would've been to print a message about suggesting to try evmVersion: 'paris' if the user gets invalid opcode error.

making the evmVersion mandatory... I think that might be too much.

It would give certainty, especially amid this flip-flopping of default EVM version. Otherwise a newcomer would naturally think they're using latest versions if they only set version: '0.8.21' in hardhat.config.js. Just as Solidity version is required, so too evmVersion could be required.

[SKYBITDev3]

You could add evmVersion explicitly into hardhat.config.js when a user creates a new hardhat project, e.g.:

  solidity: {
    compilers: [
      {
        version: `0.8.21`,
        settings: {
          optimizer: {
            enabled: true,
            runs: 1000
          },
          evmVersion: `shanghai`, // downgrade to `paris` if you encounter 'invalid opcode' error
        }
      },
    ],
  },

[fvictorio]

Adding a mandatory field to the config is a breaking change, so that's not something we are likely to do. In fact, no field in the config is mandatory right now (not even the solidity version), so it would be weird to add only that one.

if we came across invalid opcode error, we would do some searching and find that the solution is simply to downgrade by adding evmVersion: 'paris' in hardhat.config.js

Some users might realize this only after deploying. This is what we are trying to prevent.

The cost here is that users, by default, will have somewhat bigger generated bytecodes. People that care about those optimizations will likely already know how to do it. Again, this is a question of on which side do you want to err on. We decided that "completely preventing people from deploying invalid bytecode" was better than "completely preventing people from unnecessarily wasting a bit of gas during deployment".

I do agree with your main point that this should've been a minor version, not a patch. But that's not something we are going to undo now.

[SKYBITDev3]

In fact, no field in the config is mandatory right now (not even the solidity version)

I've just tried to compile without solidity in hardhat.config.js and compilation fails with many lines of error messages including "Solidity compiler is not configured". So it looks like solidity is required.

Adding a mandatory field to the config is a breaking change, so that's not something we are likely to do.

OK but even without making it mandatory you could still add more to the default hardhat.config.js when a user runs yarn hardhat init, e.g. include optimizer and evmVersion like what I suggested above. I've created #4424 for this.

Some users might realize this only after deploying. This is what we are trying to prevent.

There's no "after deploying" as the deployment fails if the blockchain doesn't support it. Then as developers we find out why it failed and work around it. Hardhat could have helped by adding some guidance when that "invalid opcode" error appears, e.g. about advising to add evmVersion in hardhat.config.js.

I do agree with your main point that this should've been a minor version, not a patch. But that's not something we are going to undo now.

No worries, I wouldn't expect you to undo it, so this issue is more for better future practice.

[fvictorio]

There's no "after deploying" as the deployment fails if the blockchain doesn't support it.

Ah, you are right. If the deployment bytecode has a PUSH0 (and it almost surely has), then it will fail. I was thinking only about the deployed bytecode.

[fvictorio]

Closing this for now. The latest version of Hardhat shows the targeted EVM(s) in the compilation output, which I think should be enough for now, but happy to re-visit this in the future.