Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use appropriate EVM version for a particular chain - eg Celo #5275

Open
ryestew opened this issue Oct 10, 2024 · 12 comments
Open

Use appropriate EVM version for a particular chain - eg Celo #5275

ryestew opened this issue Oct 10, 2024 · 12 comments
Assignees
Labels

Comments

@ryestew
Copy link
Collaborator

ryestew commented Oct 10, 2024

Some chains don't use the opcodes in the latest EVM upgrade - so in Remix, the latest EVM version, is selected by default but then when a user selects a chain, like Celo, that doesn't use some of the opcodes in the latest EVM, the contract won't deploy and you get an error like this:
image

The complicating factor is that Remix doesn't know where you'll deploy to when you are compiling. If it did, and if there was a list of chains and their latest EVM compatible version and if this list was up to date, then Remix could set the EVM when compiling to the correct version.

But also when I use the Paris EVM and deploy to Alphajores, I get an RPC error.

Describe the solution you'd like
A new workflow should be introduced where the Remix is told the chain the contract will be deploying to.

@yann300
Copy link
Contributor

yann300 commented Oct 14, 2024

Proposition of workflow:

  • We internally store a mapping chainid <=> evmVersion.
  • When a chain is switched to, we check the current chain_id. If the current selected evm_version in the solidity compiler doesn't match the one in the mapping, we automatically switch to it, and show a msg in the terminal switching the solidity compiler to a compatible evm version. Please recompile the contract.

@LianaHus
Copy link
Collaborator

I would say not only the terminal but a toaster would make even more sence

@ryestew
Copy link
Collaborator Author

ryestew commented Oct 14, 2024

@LianaHus - your idea of using the toaster seems fine - but weren't we trying to minimize toaster use? Even if we are, is this example a good one for employing a toaster?

@GigaHierz
Copy link

GigaHierz commented Oct 14, 2024

Adding the case of, when the file uses a higher version, that can't be compiled through remix.

In that case, we could have a toast that says:

"for your chain only the evm_version below is available, please downgrade your file"

Or how would you handle that?

@LianaHus
Copy link
Collaborator

@LianaHus - your idea of using the toaster seems fine - but weren't we trying to minimize toaster use? Even if we are, is this example a good one for employing a toaster?

IMHO this is when it is still appropriate to use it and yes you are right, we try to minimize it.

@LianaHus
Copy link
Collaborator

Adding the case of, when the file uses a higher version, that can't be compiled through remix.

In that case, we could have a toast that says:

"for your chain only the evm_version below is available, please downgrade your file"

Or how would you handle that?

I would switch automatically and then to inform. We switch sol version based on pragma, but the logic there should be rethinked

@ryestew
Copy link
Collaborator Author

ryestew commented Oct 15, 2024

@GigaHierz - Is your comment is in the case of a chain that uses opcodes that are not yet merged into the EVM? I ask because you say that the file uses a "higher version". Or is it in the case where soon after a merge, there could be a delay before the latest EVM is available in Remix?

@GigaHierz
Copy link

Hi @ryestew , I realize I moved into a different conversation.

In my last message, I was referring to compiler versions, as a workaround that some people proposed was to use compiler versions lower than 0.8.20 and I was referring to how it would work if we would adapt the compiler versions, in case someone chooses a higher version.

Now, going through the conversation again, I realize we are talking about the EVM version.

So actually, nothing to add to that proposal. The proposed solution sounds good to me.

One other question would actually be, what opcodes are not available on Celo and if that will change with Celo's migration to a L2. Alfajores has already been migrated to an L2 so the error message you added above is the same as I am getting when deploying on Alfajores but different from the one that I am getting when deploying to Celo Mainnet.

image

@ryestew
Copy link
Collaborator Author

ryestew commented Oct 21, 2024

@GigaHierz - so are you saying that there are two issues to deal with:

  1. When the chosen compiler version is higher than the specified compiler version in Solidity file.
  2. When a chain is missing some opcodes that are present in the latest EVM.

But I think you are also bringing up a 3rd issue - the problem of the error on Celo mainnet and the problem on Celo Alfajores - the error messages - while different from each other - don't really provide helpful info.
Please elaborate and lets schedule a call.

@GigaHierz
Copy link

hi @ryestew ,

yes agree. It definitely makes sense to separate them.

  1. When a chain is missing opcodes it should automatically switch to a compatible EVM version. and show a toast taht it has been updated.
  2. If the chosen compiler version is higher than 0.8.20 there should be a warning that you need to choose a lower version on Celo. (maybe again with a mapping)

These two workflows should catch the error messages IMO.

Will reach out to schedule a call

@ryestew
Copy link
Collaborator Author

ryestew commented Oct 29, 2024

Our original logic as that the EVM version would need to change depending on the chain.
For your point 2 above - you are saying that we need to take into account both the EVM version and the Solidity version to accomodate different chains.

@ryestew
Copy link
Collaborator Author

ryestew commented Oct 30, 2024

The Celo mainnet error for me at least now looks better (the object object is now gone)
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Needs triage
Development

No branches or pull requests

6 participants
@joeizang @ryestew @yann300 @LianaHus @GigaHierz and others