foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
7.91k stars 1.6k forks source link

Better dependency model #2618

Open onbjerg opened 1 year ago

onbjerg commented 1 year ago

Component

Forge

Describe the feature you would like

@mattsse in #2098

this will require a bit of work, basically managing a registry, but would be huge, getting rid of git submodules entirely would be amazing.

this would require

  • [ ] dependency section in foundry.toml
  • [ ] registry model for ~/.foundry/registry/...
  • [ ] some libgit2 utils to create checkouts, used some of cargo's code for that in the binder create already
  • [ ] ...

Some initial exploration is being done in #2386. This issue is a place to discuss potential features.

Also solves the following issues:

An MVP would just use git, but using libgit2. Later this can be expanded upon to support a registry model, or some package management form the community lands on

Additional context

No response

sambacha commented 1 year ago

Why not take a playbook out of nix and use the tarball model?

There is this concept from yarnv3 using the exec:// dsn: https://github.com/sambacha/foundry-exec/tree/master/plugin-exec

The exec: protocol represents a way to define yourself how the specified package should be fetched. In a sense, it can be seen as a more high-level version of the Fetcher API that Yarn provides.

This also leverages a fakeFS so its isomorphic

Ideally, you could just keep them in a zip pr tgz file and read it, etc. you still need to enforce a deterministic process for reproducing the dependency install graph (how to sort etc etc)

fubuloubu commented 1 year ago

Just gonna leave this here: https://eips.ethereum.org/EIPS/eip-2678

highlights:

difficulties:

onbjerg commented 1 year ago

@sambacha No decisions have been made on anything related to how packages are fetched outside of supporting git repositories without requiring git. Anything else would obviously require a tarball. I don't think the exec model makes sense, seems pretty insecure to allow any dependency in the subtree to just arbitrarily define a command to be run on install

sambacha commented 1 year ago

@sambacha No decisions have been made on anything related to how packages are fetched outside of supporting git repositories without requiring git. Anything else would obviously require a tarball. I don't think the exec model makes sense, seems pretty insecure to allow any dependency in the subtree to just arbitrarily define a command to be run on install

this behavior is already possible with git submodules and symlinks

onbjerg commented 1 year ago

How?

nlnw commented 1 year ago

please get away from submodules

sambacha commented 1 year ago

How?

[submodule "test"]
path = test
url = ssh://-oProxyCommand=touch VULERABLE/git@github.com:/foundry/forge-std.git

Though this has been fixed here: https://github.com/git/git/commit/0383bbb9015898cbc79abd7b64316484d7713b44#diff-07b96ecc79256b188e0ea9b2c6d1180e

CVE-2021-21300 https://nvd.nist.gov/vuln/detail/CVE-2021-21300

quote,

'As a workaound, if symbolic link support is disabled in Git (e.g. via git config --global core.symlinks false), the described attack won't work.'

Additionally for GitHub Actions, see https://blog.teddykatz.com/2022/02/23/ghosts-of-branches-past.html

GitHub's API is not a reflection of 1:1 to git, so there are probably other ways etc

brockelmore commented 1 year ago

Just gonna leave this here: https://eips.ethereum.org/EIPS/eip-2678

highlights:

* EVM native artifacts

* can publish to decentralized (really good for build reproductions) or centralized infrastructure

* no need to re-build anything once published (but can also load sources if needed, although if solc had support it would help)

* cross-language/framework artifact, already used with Vyper, Truffle, Brownie, and Ape

* Much easier integration point with external tooling, instead of replicating compile stages (e.g. Slither and crytic-compile)

* **can let you import types from non-Solidity languages like Vyper** (would be even better if Solidity completed support for it)

difficulties:

* not a lot of adoption, really only Ape is actively developing it

* v3 has some rough edges, v4 will eventually be needed

* no widely-used distribution registries

* would have to create new Rust tooling to handle the format (although started here: https://github.com/ethpm/ethpm-rs)

this eip may be the most verbose, overly-complicated EIP I've seen, and looks like a nightmare to actually use. maybe there have been learnings from this that we could take a step back and figure out minimal version with optional extensions that can come later? As it currently stands, the complexity associated with this standard seems like it would be extremely likely to introduce unintended bugs (even if the verbosity is intended to reduce such bugs).

fubuloubu commented 1 year ago

this eip may be the most verbose, overly-complicated EIP I've seen, and looks like a nightmare to actually use. maybe there have been learnings from this that we could take a step back and figure out minimal version with optional extensions that can come later? As it currently stands, the complexity associated with this standard seems like it would be extremely likely to introduce unintended bugs (even if the verbosity is intended to reduce such bugs).

It is quite complex, but most of the decisions have some basis in reality and it actually works quite nicely for our use cases (dependency management, deployment tracking, compilation caching, publishing, etc.). At least I would caution against disregarding it on the basis of "it looks complicated" since the requirements of package management are just that: complicated.

There are definitely some rough edges still to iron out, but we have for the most part identified the issues with the spec in it's current v3 form (namely import linking), and would love to start a conversation about a v4 iteration (or re-approaching the problem from scratch w/ lessons learned). I think between Foundry, Hardhat, and Ape, we can reasonably make a good standard that can improve interoperability with existing tooling (not just dev frameworks but also security tooling, Etherscan, etc.) and make code sharing across the ecosystem a lot easier (which also improves DevEx).

One thing I am not in favor of is going through and just creating a "minimal" spec that seems to meet some initial needs, because those needs will grow and change and then we'll have yet another packaging standard that only sees use with one framework, or everything designed around Solidity and however it arbitrarily decides to format it's compiler artifacts in each version. There's a lot to be gained from a little bit of foresight, just imagine how much easier it would be to work with code if you didn't have to flatten everything to get it to verify on some 3rd party platforms, but instead just put in ipfs://QmAbCd...1234#MyContractType and hit "verify"

brockelmore commented 1 year ago

alright fine you've sort of convinced me. I think ideally a small group of us cook something up, and present it in a relatively close to finished form to a few groups:

  1. Hardhat (maybe bring one of them into the design fold)
  2. Sourcify
  3. Etherscan
  4. Remix

Ideally, we get large industry buy-in from launch (Ape and Foundry obviously would be onboard from day 1). For projects like remix, sourcify, and hardhat, hopefully we find individuals dedicated enough to making this a reality to go and implement it for them, then we do a strong push to get etherscan to adopt it. Likely need a javascript lib and rust lib to facilitate quick onboarding.

sambacha commented 1 year ago

Gentlemen, we can use dpack and accretive versioning and solve all these issues.

Accretive Versioning

Accretive versioning is based on matching type signatures against a generated ABI V2.

Imagine a package manager that ran the test suite of the version you're currently using against the code of the version you'd like to upgrade to, and told you exactly what wasn't going to work.

More info: https://github.com/sambacha/dappspec/blob/master/abi/src/Accretive_Versioning.md

dpack

types is a collection of named contract types ("classes"). Each object has this form:

"MyToken": {
  "typename": "MyToken"
  "artifact": {"/": "<CID>"}
}

typename is the name of the type (ie, the Solidity class) artifacts is a DAG-JSON link to this type's "artifacts" json file (output of solc/truffle/hardhat). Note that 'typename' is redundant with key used to name this type in this pack. Typenames are mixedcase alphanumeric and underscores, but must start with an uppercase alphabetic.

The dpack format makes a clear distinction and is very explicit. You cannot name an object the same as a type.

https://github.com/dapphub/dpack

These can additionally be used via URLs securely,supporting SRI checks, etc.

sambacha commented 1 year ago

Or we can just bundle SafeMath and solve 60% of all dependency issues going forward

Merry Christmas 🎄🎁

fubuloubu commented 1 year ago

I never quite got dpack, but I think it's likely too simple for the use cases we are describing.

Main goal here is being able to share packages of code and compiled types across projects through a format where repeatable builds are possible without additional user input required (e.g. Source Code Verification)

Solidity code projects are typically too complex to represent using the simplified dpack approach

PaulRBerg commented 1 year ago

This new dependency model should come with the ability to separate development dependencies from production dependencies.

At the moment, when Foundry installs a repo, it pulls all testing-related dependencies of that repo although end users never need them.

sakulstra commented 1 year ago

Also solves the following issues:

  • A dependency on git in the users system
  • SSH vs HTTPS submodules debacle
  • Needing to teach users about submodules/providing git support
  • Issues around dependency remappings, since we have greater control of the dependencies

While i don't have any issues with git submodules per se, there are some inherent problems not noted here I started facing on projects with increasing size:

I'm not completely sold on creating a new package manager of sorts.

What would be the argument against using something existing? While I'm not the biggest fan of node ecosystem, it has been used for years now via hardhat etc. Perhaps it would be reasonable to just adopt using npm?

PaulRBerg commented 12 months ago

Perhaps it would be reasonable to just adopt using https://github.com/foundry-rs/forge-std/pull/322?

Pnpm and yes, I agree :D

fubuloubu commented 12 months ago

Supply chain attacks are rife with npm

https://portswigger.net/daily-swig/vulnerabilities-in-npm-allowed-threat-actors-to-publish-new-version-of-any-package

sakulstra commented 12 months ago

This post is from 21, the researchers who found the vulnerabilities are from GitHub(which afaik now owns/ maintains npm).

In the reality I live in I have to use npm in addition to submodules already (if it's due to misc stuff like linting, sources not hosted on github or js SDKs used to fetch off chain info - in the end we usually need sth from npm world).

Just in case this is being misread as me requesting a switch to npm. I don't.

My point is just that the current state is kinda bad and there are package managers in the wild that work(ofc each with its own quirks). So my suggestion is to perhaps use one of the available ones instead of reinventing the wheel.

fubuloubu commented 12 months ago

node_modules is still a really poor way to install code since it is an editable format, and there are additional concerns and needs that smart contract code has that other ecosystems don't such as code verification and deployment linkage which may require a fresher approach. submodules have major problems, but immutability/dependency cycles are not one of them, so let's not throw the baby out with the bathwater here

sambacha commented 11 months ago

NPM packages are tarballs

You know who else uses tarballs? Nix.

We can support retrieving deps. from npm that is not a problem. We do not, however, have to use the npm client for installing such deps. 

The current dependency model is broken. 

Also lets make a distinction between dependency management and building

sambacha commented 11 months ago

fuck this im going in

sambacha commented 10 months ago

fuck this im going in

https://asciinema.org/a/BFitn8dG0efWo6FjoC7t0uYKf

JosepBove commented 10 months ago

How's this going? It looks good

0xalpharush commented 7 months ago

Fwiw, a better dependency model may need to account for isses like (https://github.com/gakonst/ethers-rs/issues/2609) maybe by using contexts for remappings