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

Forge dependency / library install instructions #1

Closed
decafboba opened this issue Apr 17, 2022 · 6 comments
Closed

Forge dependency / library install instructions #1

decafboba opened this issue Apr 17, 2022 · 6 comments

Comments

@decafboba
Copy link

decafboba commented Apr 17, 2022

Hello @gaslimitreached, thanks for putting this repository together!

For example, the basic usage in this tutorial does:
forge install OpenZeppelin/openzeppelin-contracts

Under normal conditions this will add it to ./lib. For some reason when trying it with this template, I get:

forge install OpenZeppelin/openzeppelin-contracts
Installing openzeppelin-contracts in "<..>/foundry-hardhat-template/lib/openzeppelin-contracts", (url: https://github.com/OpenZeppelin/openzeppelin-contracts, tag: None)
Error: 
   0: A git directory for 'lib/openzeppelin-contracts' is found locally with remote(s):
        origin  https://github.com/OpenZeppelin/openzeppelin-contracts
      If you want to reuse this local git directory instead of cloning again from
        https://github.com/OpenZeppelin/openzeppelin-contracts
      use the '--force' option. If the local git directory is not the correct repo
      or you are unsure what this means choose another name with the '--name' option.

Is there some special instructions for adding dependencies / libs with Foundry such that we can use them in forge test for unit/fuzzing tests?

Also, curious on when we should be duplicating the dependencies with yarn/npm. For example, if our contracts have a dependancy on OpenZeppelin contracts, should we do both yarn add @openzeppelin/contracts and forge install OpenZeppelin/openzeppelin-contracts?

Thanks again for your help, this is the best forge + hardhat I've found.

@gaslimitreached
Copy link
Owner

Hey, thanks for bringing this up as I really need to revisit this readme. The workflow I use is to use yarn to add the dependency and then adding the node modules path to foundry.toml and remappings.txt.

Something like: @openzeppelin/contracts=node_modules/@openzeppelin/contracts
and import like: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";

I'm leaving this issue open as reminder to myself to update the docs. Please let me know if you run into trouble with the above.

@decafboba
Copy link
Author

This worked, thank you!

I do still have some questions in regard to dependencies with this setup:

  1. Should we consolidate the remappings into one? Right now we have separate lists in foundry.toml and remappings.txt, and it seems like maybe it would be a bit cleaner if there was just one source of truth.
  2. With the rari-capital/solmate example, I see that it is still in the ./lib dir even though it is also being added in package.json. Since OpenZepplin seems to work without it being added to ./lib, maybe it is better to just completely remove these dependencies from ./lib and .gitsubmodules?
  3. If (2) is possible, then it would just be ds-test and forge-std in ./lib. Maybe we could move everything as package.json / npm dependencies, then completely remove ./lib and .gitsubmodules?

I think these changes would make things a lot cleaner if they are possible, since it would consolidate the dependency management down to single sources of truth from double. But not sure on if it's fully possible.

@gaslimitreached
Copy link
Owner

  1. I've had this same question but not a good answer. I have noticed that if I remove remappings.txt then vscode has problems finding the deps.
    2 & 3. Agree. The limitation is realized when the library doesn't have an equivalent npm package which is the case ds-test and forge-std.

Thanks for this. I really appreciate the feedback and I'm excited to find out someone is using it.

@gaslimitreached
Copy link
Owner

Thanks again for your feedback. I added a note about dependencies and cleaned up the lib directory as you suggested.

@decafboba
Copy link
Author

I let the Forge folks know, and hopefully they can just wrap their repo as NPM modules, so we can tidy this up.

@decafboba
Copy link
Author

forge-std is up: foundry-rs/forge-std#39 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants