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

request: forge script build with full artifacts for vm.getDeployedCode to work #7732

Closed
protolambda opened this issue Apr 19, 2024 · 0 comments · Fixed by #7738
Closed

request: forge script build with full artifacts for vm.getDeployedCode to work #7732

protolambda opened this issue Apr 19, 2024 · 0 comments · Fixed by #7738
Labels
T-feature Type: feature

Comments

@protolambda
Copy link

protolambda commented Apr 19, 2024

Component

Forge

Describe the feature you would like

Background: forge test and forge script behavior difference

forge script is implemented such that it targets a specific contract with:

let output = compile::compile_target(&target_path, &project, args.opts.silent)?;

forge test is implemented such that it compiles the whole project with:

let sources_to_compile = self.get_sources_to_compile(&config, &filter)?;

let compiler = ProjectCompiler::new()
    .quiet_if(self.json || self.opts.silent)
    .files(sources_to_compile);

let output = compiler.compile(&project)?;

Note that get_sources_to_compile was modified in #7572 to resolve #7569

Now I am facing the same kind of issue as in the above force test fix, but in forge script: the vm.getCode and vm.getDeployedCode cheatcodes only allow you to load artifacts from the current build.

This forge test approach allows you to load a contract-bytecode of a contract that isn't imported directly in the script, to then do vm.etch and execute the contract. This is very important, since it allows conflicting solidity versions to interoperate, and it avoids large bytecodes (if importing and deploying without vm.etch there is a lot of overhead). In the OP-Stack we are using foundry-scripts to perform large deployments, and now also L2 genesis-state creation, and this functionality of forge test we would really like to have in forge script also.

What I have tried

When using unchecked_cheatcode_artifacts = true, to access deployments of older builds, there is no automated dependency resolution (if there are multiple solidity versions of build output), and arbitrary stale artifact reading isn't great either.

When importing the solidity files that I am calling vm.getDeployedCode for, the solidity version conflicts arise, preventing me from compiling at all.

When using absolute paths (instead of the name.sol:name syntax) to the artifacts in vm.getDeployedCode, the checks for artifacts are still the same, and compiler output is not consistent even if it worked.

What I am looking for is to make vm.getDeployedCode to work the same in forge script as forge test. If this is behind a flag that's fine, e.g. --full-scope or something to build the whole project.

Implementation

Forge test compiler invocation:

let output = compiler.compile(&project)?;

Forge script compiler invocation:

let output = compile::compile_target(&target_path, &project, args.opts.silent)?;

I think that by pulling in the ProjectCompiler (optionally, if there is a difference in speed), creating a full build output with all artifacts, and then proceeding with the same artifact-filtering to identify the script-contract to execute, we can run the script with expected vm.getCode and vm.getDeployedCode behavior.

Additional context

No response

@protolambda protolambda added the T-feature Type: feature label Apr 19, 2024
tynes added a commit to ethereum-optimism/optimism that referenced this issue Apr 22, 2024
Updates foundry to `63fff3510408b552f11efb8196f48cfe6c1da664`
which includes foundry-rs/foundry#7738.
This fixes the issue foundry-rs/foundry#7732
which was preventing #10106
from just working.

Props to the foundry devs for fixing our issues very quickly, unblocking
our ability to ship.

Need to follow up with a bump to `ci-builder` such that it includes this
release of foundry and then rebase #10106 on top so that it can pass
tests.
github-merge-queue bot pushed a commit to ethereum-optimism/optimism that referenced this issue Apr 22, 2024
Updates foundry to `63fff3510408b552f11efb8196f48cfe6c1da664`
which includes foundry-rs/foundry#7738.
This fixes the issue foundry-rs/foundry#7732
which was preventing #10106
from just working.

Props to the foundry devs for fixing our issues very quickly, unblocking
our ability to ship.

Need to follow up with a bump to `ci-builder` such that it includes this
release of foundry and then rebase #10106 on top so that it can pass
tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-feature Type: feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant