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

feat: add way for files to import other sources to work with vm.getCode #7025

Open
adhusson opened this issue Feb 6, 2024 · 5 comments
Open
Labels
A-config Area: config T-feature Type: feature T-to-discuss Type: requires discussion

Comments

@adhusson
Copy link
Contributor

adhusson commented Feb 6, 2024

Component

Forge

Describe the feature you would like

tldr

The effect of // forge-import: Contract.sol is to add a contract to the list of contracts to be compiled (but it does not import it in the file containing the directive).

Rationale

vm.getCode("Contract.sol") can be used to import the bytecode of a contract that was compiled using a different compiler version (to match an onchain deployment for instance). If the contract is not used anywhere else, an additional mock file can be used to make sure the contract is compiled by forge build.

This stops working when the foundry project that uses getCode is itself used as a dependency. The mock file is usually not imported, so Contract.sol is not compiled at all.

The forge-import directive would statically add a contract to the compilation plan.

Example

Before

// src/MockUniswapV3Factory.sol
pragma solidity 0.7.6;
import "v3-core/UniswapV3Factory.sol";
// test/Fixture.sol
pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "v3-core/interfaces/IUniswapV3Factory.sol";
contract MyTest is Test {
    IUniswapV3Factory internal uniswapV3Factory;

    constructor() {
        uniswapV3Factory = IUniswapV3Factory(deployCode("UniswapV3Factory.sol"));
    }
}

After

// test/Fixture.sol
pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "v3-core/interfaces/IUniswapV3Factory.sol";
contract MyTest is Test {
    IUniswapV3Factory internal uniswapV3Factory;

    constructor() {
        // forge-import: UniswapV3Factory.sol
        uniswapV3Factory = IUniswapV3Factory(deployCode("UniswapV3Factory.sol"));
    }
}

Related: #6099

@adhusson adhusson added the T-feature Type: feature label Feb 6, 2024
@gakonst gakonst added this to Foundry Feb 6, 2024
@github-project-automation github-project-automation bot moved this to Todo in Foundry Feb 6, 2024
@sambacha
Copy link
Contributor

sambacha commented Feb 8, 2024

Please no, this feels like more like a foundry.toml thingy than a comment directive. 

People will use this for more than this stated use case, and forge has yet to address its dependency management technical debt.

@adhusson
Copy link
Contributor Author

adhusson commented Feb 8, 2024

What other use cases do you have in mind? Definitely possible to add a foundry.toml option instead, but having the import right next to the getCode call looks easier to maintain.

@zerosnacks
Copy link
Member

Not a fan of the proposed comment directive approach but I can see how this feature could be useful, echoing #7738

Perhaps a mapping similar to how libraries can be linked in foundry.toml?:

imports = [
  "path/to/file.sol:ContractName"
]

and then you can reference the artifact by name

cc @klkvr / @mattsse

@zerosnacks zerosnacks changed the title Add a // forge-import: Contract.sol comment directive feat: add way for files to import other sources to work with vm.getCode Jul 10, 2024
@zerosnacks zerosnacks added this to the v1.0.0 milestone Jul 26, 2024
@grandizzy
Copy link
Collaborator

@adhusson would the proposed PR #8668 solve the scenario you're looking for?

@adhusson
Copy link
Contributor Author

adhusson commented Oct 3, 2024

AFAICT no as it does not force the compilation of additional files (instead it discriminates files already scheduled for compilation).

Combined with @zerosnacks's suggestion yes it would help.

@grandizzy grandizzy removed this from the v1.0.0 milestone Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-config Area: config T-feature Type: feature T-to-discuss Type: requires discussion
Projects
Archived in project
Development

No branches or pull requests

4 participants