Support context aware remappings from nested foundry imports

[mattsse]

Component

Forge

Describe the feature you would like

Currently forge picks up remappings from imported libraries as well, if a nested library and the project use the same remapping but for different targets (different version of the same dependency for example), the current behaviour is that the project's remapping overrides the nested lib's remapping, which may not be intended.

solc remappings support a context: prefix to circumvent this:

https://docs.soliditylang.org/en/latest/path-resolution.html#import-remapping

context:prefix=target

where context must match the beginning of the source unit name of the file containing the import.

so that solc can be invoked like

solc module1:github.com/ethereum/dapp-bin/=dapp-bin/ \
     module2:github.com/ethereum/dapp-bin/=dapp-bin_old/ \
     source.sol

This would require some upstream changes in ethers-rs:

• add optional context field to Remapping

this would also require us to track where the Remapping is coming from so we can derive the context (which should be the relative path lib/<dep>/ I think)

Additional context

No response

[gakonst]

TIL context:

[fredlacs]

Is anyone actively working on this issue?
Working on a project that needs multiple solidity versions on different dependencies and blocked on this feature

[gakonst]

@fredlacs can you share a repro case which illustrates teh bug?

[fredlacs]

Here is a minimal repro of what I'm running into https://github.com/fredlacs/multiple-solc-repro
If you switch the commented out imports in src/*Dependencies.sol you can't make it compile unless you change the packages imported in the submodules

I'm trying to manage 2 dependencies, one using solc 0.6 and another one using 0.8

I've seen other projects just manage multiple OZ versions locally (as in https://github.com/Phission-Finance/Phission-Finance/tree/master/lib).

Now that I think of it a bit more maybe the solution here isn't the context: feature but rather a "deep remapping" to specify what to link @openzeppelin/contracts to in each dependency

[dalechyn]

affected by this as well.
importing thirdweb and oz contracts.

[cxkoda]

Same when building around existing contracts. Having transitive dependency resolution would be great.

[gakonst]

Can you guys show some minimal repros for us to better understand the problem?

[cxkoda]

See #2693 for an example

[gakonst]

See #2693 for an example

@mattsse can u ptal

[Rubilmax]

Context-specific imports would be very helpful to avoid version incompatibilities in nested dependencies (and thus increase submodule composability)

[Rubilmax]

Hey @mattsse, any plans to implement this?

We've got an issue @morpholabs which we can illustrate with the following, smallest git setup:

Let repository dependency depend on openzeppelin-contracts & have remappings.txt:

@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts

Let repository module depend on dependency

Then module MUST have openzeppelin-contracts as dependency, but no commit restriction is applied, which allows module to install any version of openzeppelin-contracts, including ones that may not be compatible with dependency!

One way of solving this could be to have forge compile with nested submodules instead of module's root submodules (thus, the commit restriction would be applied by each git repository)

[mattsse]

@Rubilmax

I'll take a closer look now.

I think adding this should be doable with medium effort, since solc should do the heavy lifting anyways.

perhaps we start by making this an opt-in setting in forge.

[MerlinEgalite]

It would be awesome thx!

[Yorkemartin]

I ran into this issue while trying to implement opensea's blocklist. Installing operator-filter-registry as a submodule breaks OpenZepplin imports by expecting lib/openzeppelin-contracts/contracts, when our project uses lib/openzeppelin-contracts/

[joaquinlpereyra]

Affected by this as well. We are creating a repository similar to our learn-evm-attacks project.

The main thing is that we are not writing many contracts but we do write a lot of tests. When we add a new repository-to-test, the ideal workflow would be to simply

$ forge install [project-to-test]

Unfortunately due to this problem, this is near impossible: different projects will use different versions and remappings. Context aware remappings are a must here for us.

[PaulRBerg]

Do you happen to have any updates on this, @mattsse? Sorry to tag you, but I've just bumped into a similar scenario to @joaquinlpereyra.

It seems to me that the more successful Foundry becomes, the more important this feature becomes because Foundry projects become more intertwined (both depth-wise and breadth-wise).

Our project sablier-labs/v2-periphery has many remappings:

https://github.com/sablier-labs/v2-periphery/blob/76c5af55739c0447bff956ea6287be3ae8e8c22a/remappings.txt

Because of that and the lack of context-aware remappings in Foundry, V2 Periphery itself cannot be consumed by any third-party Foundry repo (without a patch). I installed v2-periphery in my foundry-template, trying to import something from src/types/Permit2.sol, and got this error:

Failed to resolve file: "/Users/prb/Workspace/templates/foundry-template/lib/v2-periphery/lib/permit2/interfaces/IAllowanceTransfer.sol": No such file or directory (os error 2).
 Check configured remappings..
    --> "/Users/prb/Workspace/templates/foundry-template/lib/v2-periphery/src/types/Permit2.sol"
        "permit2/interfaces/IAllowanceTransfer.sol"

Then, I ran forge remappings, and got this:

See remappings output

@openzeppelin/contracts/=lib/v2-periphery/lib/openzeppelin-contracts/contracts/
@prb/math/=lib/v2-periphery/lib/v2-core/lib/prb-math/src/
@prb/proxy-test/=lib/v2-periphery/lib/prb-proxy/test/
@prb/proxy/=lib/v2-periphery/lib/prb-proxy/src/
@prb/test/=lib/prb-test/src/
@sablier/v2-core-script/=lib/v2-periphery/lib/v2-core/script/
@sablier/v2-core-test/=lib/v2-periphery/lib/v2-core/test/
@sablier/v2-core/=lib/v2-periphery/lib/v2-core/src/
@sablier/v2-periphery/=lib/v2-periphery/src/
ds-test/=lib/forge-std/lib/ds-test/src/
erc4626-tests/=lib/v2-periphery/lib/openzeppelin-contracts/lib/erc4626-tests/
forge-gas-snapshot/=lib/v2-periphery/lib/permit2/lib/forge-gas-snapshot/src/
forge-std/=lib/forge-std/src/
openzeppelin-contracts/=lib/v2-periphery/lib/openzeppelin-contracts/
openzeppelin/=lib/v2-periphery/lib/openzeppelin-contracts/contracts/
permit2-test/=lib/v2-periphery/lib/permit2/test/
permit2/=lib/v2-periphery/lib/permit2/
prb-math/=lib/v2-periphery/lib/v2-core/lib/prb-math/src/
prb-proxy/=lib/v2-periphery/lib/prb-proxy/
prb-test/=lib/prb-test/src/
solady/=lib/v2-periphery/lib/solady/
solarray/=lib/v2-periphery/lib/v2-core/lib/solarray/src/
solmate/=lib/v2-periphery/lib/permit2/lib/solmate/src/
src/=src/
v2-core/=lib/v2-periphery/lib/v2-core/
v2-periphery/=lib/v2-periphery/

Notice the permit2 remapping, which is the problem here. Foundry does not pick our own permit2 remapping, and as a result, the inferred remapping lacks the /src suffix (and I get the error shared above).

We can hot-fix this by asking consuming repos to add an explicit re-remapping, like this:

permit2/=lib/v2-periphery/lib/permit2/src/

But this is a chore - for us and our users - and it's not scalable either (what happens when we have another dependency?).

[PaulRBerg]

Another hot-fix would be to rename the permit2 remapping to smth like @uniswap/permit2. However, this would be another non-scalable approach because it assumes that all dependencies in the lib tree have remappings that do not match the submodule names. Requiring users to do this in perpetuity would be a chore and a footgun.

[mattsse]

this would definitely be a big QoL upgrade and lacking support is becoming more and more painful.

let's tackle this shortly @Evalir

[onbjerg]

Hi guys,

I am looking for up to date minimal repros to validate #5397. I've tested the repository in #3440.

Lmk

[PaulRBerg]

I am looking for up to date minimal repros to validate #5397.

You could try sablier-labs/examples at this commit: eae781627e15d963a62c1545fe0944ad94d5519c.

[SweetmanTech]

Is this a correct way to override mappings from /lib/creator-token-contracts/ context?

lib/creator-token-contracts:@openzeppelin/=lib/openzeppelin-contracts/

here's the remappings in /lib/creator-token-contracts/remappings.txt

@openzeppelin/=node_modules/@openzeppelin/
ds-test/=lib/forge-std/lib/ds-test/src/
forge-std/=lib/forge-std/src/
hardhat/=node_modules/hardhat/
murky/=lib/murky/src

we want the creator-token-contracts to use our higher level remapping of OpenZeppelin

higher level remappings

ERC721A-Upgradeable/=lib/ERC721A-Upgradeable/contracts/
ERC721A/=lib/ERC721A/contracts/
erc721a/=lib/ERC721A/
ds-test/=lib/forge-std/lib/ds-test/src/
erc4626-tests/=lib/openzeppelin-contracts/lib/erc4626-tests/
forge-std/=lib/forge-std/src/
ERC721C/=lib/creator-token-contracts/contracts/
openzeppelin-contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/
openzeppelin-contracts/=lib/openzeppelin-contracts/contracts/
@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts/
@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/

QUESTION: IS THIS MAPPING CORRECT?
lib/creator-token-contracts:@openzeppelin/=lib/openzeppelin-contracts/

works locally, but, in Github Actions we see

[PaulRBerg]

@SweetmanTech see #5447, it may be related to your issue.