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: improve dependency model / package management #2618

Open
onbjerg opened this issue Aug 4, 2022 · 24 comments
Open

feat: improve dependency model / package management #2618

onbjerg opened this issue Aug 4, 2022 · 24 comments
Labels
A-extensions Area: extensions C-forge Command: forge Cmd-forge-pm Command: forge install/update/remove T-feature Type: feature

Comments

@onbjerg
Copy link
Member

onbjerg commented Aug 4, 2022

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:

  • 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

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

@onbjerg onbjerg added T-feature Type: feature C-forge Command: forge Cmd-forge-pm Command: forge install/update/remove labels Aug 4, 2022
@onbjerg onbjerg linked a pull request Aug 10, 2022 that will close this issue
3 tasks
@sambacha
Copy link
Contributor

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
Copy link

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)

@onbjerg
Copy link
Member Author

onbjerg commented Aug 19, 2022

@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
Copy link
Contributor

@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
Copy link
Member Author

onbjerg commented Aug 20, 2022

How?

@nlnw
Copy link

nlnw commented Sep 10, 2022

please get away from submodules

@sambacha
Copy link
Contributor

How?

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

Though this has been fixed here:
git/git@0383bbb#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
Copy link
Member

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
Copy link

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
Copy link
Member

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
Copy link
Contributor

sambacha commented Dec 21, 2022

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

  • thanks Nikolai,

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

@sambacha
Copy link
Contributor

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

Merry Christmas 🎄🎁

@fubuloubu
Copy link

fubuloubu commented Dec 21, 2022

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
Copy link
Contributor

PaulRBerg commented Apr 27, 2023

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
Copy link
Contributor

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:

  • dependency duplication (& version mis-matches)
  • infinite circular dependency loops
  • insanely huge repos (caused by the above). It's quite common to have repos with 2gb+

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
Copy link
Contributor

Perhaps it would be reasonable to just adopt using foundry-rs/forge-std#322?

Pnpm and yes, I agree :D

@fubuloubu
Copy link

@sakulstra
Copy link
Contributor

sakulstra commented Jul 21, 2023

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
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

sambacha commented Aug 24, 2023

fuck this im going in

@sambacha
Copy link
Contributor

fuck this im going in

https://asciinema.org/a/BFitn8dG0efWo6FjoC7t0uYKf

@JosepBove
Copy link

How's this going? It looks good

@0xalpharush
Copy link
Contributor

Fwiw, a better dependency model may need to account for isses like (gakonst/ethers-rs#2609) maybe by using contexts for remappings

@zerosnacks zerosnacks added the A-extensions Area: extensions label Jun 26, 2024
@zerosnacks zerosnacks added this to the v1.0.0 milestone Jul 26, 2024
@zerosnacks zerosnacks changed the title Better dependency model feat: improve dependency model / package management Aug 1, 2024
@zerosnacks zerosnacks removed this from the v1.0.0 milestone Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-extensions Area: extensions C-forge Command: forge Cmd-forge-pm Command: forge install/update/remove T-feature Type: feature
Projects
No open projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

10 participants