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

Ignore EVM errors when staging contracts #1693

Closed
joshuahannan opened this issue Aug 12, 2024 · 3 comments · Fixed by #1697
Closed

Ignore EVM errors when staging contracts #1693

joshuahannan opened this issue Aug 12, 2024 · 3 comments · Fixed by #1697
Assignees
Labels
Feature A new user feature or a new package API

Comments

@joshuahannan
Copy link
Member

joshuahannan commented Aug 12, 2024

Issue To Be Solved

When staging contract code that relies on the EVM contract, the staging reports errors about InternalEVM that should not be reported.

error: cannot find variable in this scope: `InternalEVM`
   --> 8c5303eaa26202d6.EVM:513:19
    |
513 |         let addr = InternalEVM.createCadenceOwnedAccount(uuid: acc.uuid)
    |                    ^^^^^^^^^^^ not found in this scope

error: cannot find variable in this scope: `InternalEVM`
   --> 8c5303eaa26202d6.EVM:524:15
    |
524 |         return InternalEVM.run(
    |                ^^^^^^^^^^^ not found in this scope

Suggest A Solution

Ignore the errors about the EVM contract.

Context

Some contracts that are trying to stage will show this error and it could mask other actual errors

@joshuahannan joshuahannan added the Feature A new user feature or a new package API label Aug 12, 2024
@gregsantos gregsantos moved this to 🆕 New in 🌊 Flow 4D Aug 14, 2024
@nvdtf
Copy link
Member

nvdtf commented Aug 14, 2024

Related onflow/cadence-tools#331
@jribbink I see the above is closed, is there further integration needed here?

@jribbink
Copy link
Contributor

jribbink commented Aug 14, 2024

Related onflow/cadence-tools#331 @jribbink I see the above is closed, is there further integration needed here?

Yeah, this is a similar fix, fairly trivial effort. The LS & C1.0 update validator don't share the same codebase (there's some overlap, but was not really worth the effort to refactor given this tool is short-lived).

The caveat is that the fix for the LS exposes the InternalEVM contract to all locations. There's some added complexity to providing an alternate stdlib based on contract location.

A risk to making a similar fix here is that if someone erroneously uses the InternalEVM contract in their contract, the validator could give a false positive result (I don't know why they would do this/if this is an actual concern).

@jribbink jribbink self-assigned this Aug 14, 2024
@jribbink jribbink moved this from 🆕 New to 🏗 In Progress in 🌊 Flow 4D Aug 14, 2024
@jribbink
Copy link
Contributor

Looked into it and I was able to do this using the evm.SetupEnvironment function, so the InternalEVM contract is only available to the EVM contract now and nowhere else.

#1697

@jribbink jribbink moved this from 🏗 In Progress to 👀 In Review in 🌊 Flow 4D Aug 15, 2024
@github-project-automation github-project-automation bot moved this from 👀 In Review to ✅ Done in 🌊 Flow 4D Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature A new user feature or a new package API
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants