morpho-org / morpho-utils

Repository gathering useful libraries and contracts.
GNU Affero General Public License v3.0
65 stars 1 forks source link

Avoid dependency version incompatibilities at compilation #79

Closed Rubilmax closed 1 year ago

Rubilmax commented 1 year ago

Currently, forge compiles with the paths defined in remappings.txt defined at a project's root. This is the source of compilation incompatibilities (sometimes even silent!) because forge does not follow nested remappings.txt in submodules, which can lead to unexpected compilation results

For example, if lib/openzeppelin-contracts/'s commit does not match lib/morpho-utils/lib/openzeppelin-contracts/'s, we could get a compilation error or even a successful compilation, with a single openzeppelin-contracts version used along with morpho-utils which wasn't necessarily built for:

/project
| lib/
|     morpho-utils/
|                  lib/
|                      openzeppelin-contracts/
|     openzeppelin-contracts/
| remappings.txt
|               @morpho-utils=lib/morpho-utils/
|               @openzeppelin/contracts=lib/openzeppelin-contracts/contracts/

Possible solutions

  1. copy-paste dependencies into the project 1.a. dependencies need to be updated by hand (is a pro & a con)
  2. use relative imports rather than absolute, such as ../lib/openzeppelin-contracts/contracts/Dependency.sol instead of @openzeppelin/contracts/Dependency.sol
  3. prefix imports with context-specific naming (name @project/openzeppelin/contracts/Dependency.sol instead of @openzeppelin/contracts/Dependency.sol) 3.a. requires root remappings.txt to map all the nested dependencies (is already the case, but is currently masked because dependency paths are shared through remappings)
MerlinEgalite commented 1 year ago

I think you can ask on the forge/foundry telegram group if there's a solution

Rubilmax commented 1 year ago

A more solution-specific version of this issue is available here: https://github.com/morpho-dao/morpho-data-structures/issues/102

Rubilmax commented 1 year ago

This issue is related to https://github.com/foundry-rs/foundry/issues/1855

MerlinEgalite commented 1 year ago

I think I prefer the 3rd solution namely:

prefix imports with context-specific naming (name @morpho-utils/openzeppelin/contracts/Dependency.sol instead of @openzeppelin/contracts/Dependency.sol) requires root remappings.txt to map all the nested dependencies (is already the case, but is currently masked because dependency paths are shared through remappings)

As it allows to avoid duplication by hand (which can be annoying and prone to error) and it's rather easy to manage.

Rubilmax commented 1 year ago

But solution 3) breaks compatibility with hardhat (or at least require the developer to add remappings.txt with all the remappings from the root + add a script to read remappings.txt when compiling with hardhat)

MathisGD commented 1 year ago

I think that this is not an issue here right ? Can we close it ?

Rubilmax commented 1 year ago

Closed in favor of https://github.com/morpho-dao/morpho-data-structures/issues/102