feature(forc-pkg): Support IPFS-sourced dependencies #4299
Conversation
mitchmindtree
added
enhancement
kayagokalp
force-pushed
the
mitchmindtree/forc-pkg-ipfs
branch
from
fd35715
to
b42ceee
Compare
kayagokalp
changed the title
There was a problem hiding this comment.
Great stuff @kayagokalp! Super exciting to see this working locally :)
While the local node seemed to work smoothly (via ipfs-api), when falling back to the gateway I appeared to have some problems:
$ forc build
Fetching test_lib ipfs+QmQFGz4Z7gqN79PEYakpMNQSf9j58q5rcmdPbMUZ5PW9wn
Couldn't fetch from local ipfs node, reason:
hyper client error `error trying to connect: tcp connect error: Connection refused (os error 111)`
Caused by:
0: error trying to connect: tcp connect error: Connection refused (os error 111)
1: tcp connect error: Connection refused (os error 111)
2: Connection refused (os error 111).
Falling back to https://ipfs.io
# 5-minute wait
Error: failed to iterate over archiveI first get the warning about falling back to the gateway, then a 5-minute hang, and then finally the failed to iterate over archive message. Any ideas what might be going on with the error?
W.r.t. the warning, Rather than printing the full warning and stack trace in yellow, I think it's fine if we just have a single line that indicates the switch over to using the gateway, as this is potentially expected behaviour that we want to support. I'm thinking the verbose yellow warnings might scare the user into thinking something has gone wrong, or potentially also clutter their output? It would be great if we could keep the alignment/style of formatting consistent here too.
On another note kind of related to the inline comment - I haven't looked to closely at our newly added dependencies yet, but it would be great to try and make sure we use the minimal set of features where possible, and make sure that in the case a crate offers multiple backends (e.g. tokio, actix, hyper, something else) we try to pick those that are shared to minimize the number of dependencies we introduce.
Again, wicked stuff, I think using the local ipfs node via the http API like this will get us a long way before needing to embed a node into forc ourselves 👍
|
Hey @mitchmindtree, Are you pinning the package ( I came across this issue as well, if your 5001 port is not open, other nodes cannot discover your pinned stuff. I remember reading it from here. That being said we definitely, need to have better output to let the user know we are waiting a response from public gateway. The error message you see there occurred since gateway returned a 504, and forc tried to decode that response as a package. If you don't want to play with your local network firewall & ports just to test this, I have a test server running with that port open. That server hosts an ipfs node which pins a library package with CID: For dependencies I might have some unnecessary stuff since I had lots of trials so I will have a second look. I will also take care of the warning message 👍 |
|
This is super cool! A few questions since I don't have full context on this.
It would also be helpful to add to the PR description details about how you manually tested different scenarios so I could try it myself:
Some failure cases I'd like to try:
|
|
Hey @sdankel, thanks for the comment & questions!
IPFS support is an alternative way for dependencies (rather than a replacement) so there is no fallback mechanism. As there is no way 2 & 3.
One thing to note that is, with this PR, forc does not start a local node automatically, as we couldn't find a good, embeddable rust ipfs node implementation. So instead forc tries to find a local node running (by looking to the default port), if it can find the node, forc will do the fetch operation using the local node. If there is no node running in the system, forc fallbacks into using a public gateway, which is https://ipfs.io. So how the local node selects is port and how and when it stops running depends on the user and the implementation they are using. The most well known implementation is with go and it is called kubo. Also there is a js implementation that is widely popular. So as a user you can choose to use any of them or you can just rely on public gateway if you don't want to have a local node.
Of course! let me time them in different scenarios and add it to description.
This again changes with the implementation you are using, this PRs effect on forc is negligible as forc is simply a client to this stack.
That is the nice thing about public gateway, you don't! Since it is read only anyone on the internet can go and use the gateway. For example here is a link to a random file using ipfs.io public gateway. https://ipfs.io/ipfs/QmcKi2ae3uGb1kBg1yBpsuwoVqfmcByNdMiZ2pukxyLWD8/ So as a forc user you don't do anything special. You just need to provide the CID under the dependencies section in manifest file, and forc will fetch it using your local node, and if it does not exists, forc will get it using ipfs.io without any authentication.
Just a unit test for parsing. Most importantly we have 0 e2e test for fetching. This is the tricky part, since we don't have a IPFS node operated by Fuel yet, we cannot introduce a test fetching a package. Because to be able to fetch a file at least one node needs to be pinning it. That would mean I would have to let my local node running indefinitely for entire sway repo CI to work 😄. So I left full e2e tests to the end since we will eventually get to a point, where we would also have the publishing infra as a forc plugin. Once we got that far, we will likely host an IPFS node as Fuel so that every package published by our users are pinned at least once by us, this would also mean we will have packages pinned by a node that we operate/rely on which would enable the e2e tests for whole fetch operations.
I don't think anything special needed for this PR, but in general if we have the resources that would be great! I don't have a windows machine but I work on a mac and linux workstation. So I tested on those two.
Of course, To test with a local node:
[dependencies]
test_lib = { ipfs = "QmfZ3uH7dFEDkYN5RQfyu4m7L8uk8kGiLkNwzHqsrormSj" }
To test public gateway fallback:
[dependencies]
test_lib = { ipfs = "QmfZ3uH7dFEDkYN5RQfyu4m7L8uk8kGiLkNwzHqsrormSj" }
Like i mentioned in the questions above, IPFS sources are not related with git sources, so there is no fallback to git. I guess there is nothing to test in that sense but if you do not specify
This depends on your local ipfs node choice, it is handled by them so it is hard to say something. But as vague as it gets ipfs has a standard which by default starts the local node gateway in 5001 port. You can configure your node to use a different port. Which we will support once we have: #4550
I feel like you can test this with cutting your internet and shutdown your local ipfs node if it is running, but not sure will look into this but since we cannot stop public gateway i feel like it is the only way we can test this.
I am a little lost about these scenarios, can you provide an example for this? |
I was thinking if you specify a dependency version that is incompatible with another dependency, or a version that doesn't exist. This is not really related to the IPSF change though, and is hopefully already handled by forc. |
There was a problem hiding this comment.
Thanks for adding the testing details. Do you have any other packages in the public IPFS node that I could test with?
A few thoughts:
UX
We should do more to match the CLI UX of the rest of forc.
This could look like:
Fetching test_lib ipfs+QmfZ3uH7dFEDkYN5RQfyu4m7L8uk8kGiLkNwzHqsrormSj
Checking for local IPFS node
Fetching from https://ipfs.io Note: this could take several minutes.
Unpacking IPFS archive
with "Fetching", "Checking", and "Unpacking" highlighted green. Also, if there are any other intermediate status messages we could print after Fetching from https://ipfs.io, rather than just having it hang for several minutes, we should print them.
Latency
I'm a little concerned about how long it takes to fetch from IPFS. Do we know where that latency is going? If I had 10 dependencies to fetch from IPFS, would they be fetched in parallel or would it take 10+ minutes to fetch them all? We should do this kind of stress testing.
Specifying versions
I'm not sure I understand how to specify a pinned version and have it fetch from IPFS. If want to fetch test_lib v1.0.0. do I need to know the IPFS hash string to include it in my project?
I tried this:
test_lib = { ipfs = "QmfZ3uH7dFEDkYN5RQfyu4m7L8uk8kGiLkNwzHqsrormSj", tag = "v1.0.0" }
and
test_lib = { ipfs = "QmfZ3uH7dFEDkYN5RQfyu4m7L8uk8kGiLkNwzHqsrormSj", tag = "v1.0.1" }
Both seemed to succeed, but I couldn't tell if it was actually using a different version or not. Is the version tag still honored with IPFS? If not, should forc throw an error when both tag and ipfs are specified in the Forc.toml?
How does a user get the IPFS string for the version they want to use? Is there a GUI to search IPFS packages? I couldn't find it with a google search...
Thanks for the review 👌 I don't have any other package pinned my ipfs node, but can do that tomorrow, or if you have a local node running you can pin with UXSounds good! I did not go that way to prevent showing duplicate fetching statements for the same package but if it feels more cohesive that way will make the changes 👌 LatencyThis greatly depends on the network itself and there is nothing we can do about it, except for hoping it to get a lot better once we have our own hosted IPFS. That should greatly improve the latency as our own node will be pinning all the packages itself. Thus it won't have to look it up from peers. That is something we can see once we have the node operational. My initial tests felt a lot faster than ipfs.io but this is a server that I am hosting in my local network with very minimal hardware/bandwidth specs. Specifying versionsTag is something git specific, so it does not change anything with the ipfs source. In IPFS, each different package (and thus each different versions of the same package) will have a different CID. I would suggest taking a peek at the general design at #3752. You can see from the design that we are not going to be specifying a package directly once we have all the components ready (It will be still possible, but it wouldn't be preferred as much as it is pretty manual). Instead we will specify the address of the publisher and desired version and after that forc will resolve the CID from the repo contract. Basically this is the barebones of ipfs support for forc, first of many PRs to come. The implementation is split into multiple parts so that it can be reviewed 😅 Example: foo = { publisher = "mindtree", version = "1" }On a separate note we should generate an error, if users provide git specific stuff with path or IPFS sources to prevent this type of stuff happening with users. I will open an issue for it. |
|
I just took a final quick look - LGTM! Again, nice work on this Kaya :) Looking forward to seeing where you take this registry! W.r.t. the versions discussion, just wanted to echo Kaya's point about most folks likely not using
@kayagokalp I'd recommend aiming to avoid the need for including In the design, the idea of having a This blogpost is a nice insight into why cargo's flat namespace is a feature (and not a bug). There are likely better ways of dealing with name-squatting issues, and lessons to be learned from folks dealing with similar problems (e.g. ENS and |
kayagokalp
force-pushed
the
mitchmindtree/forc-pkg-ipfs
branch
from
37270bb
to
fbda510
Compare
Hmm that is weird., are you sure you are building this PR and using the freshly built binary? You might have a ❯ fuel/dev/sway/target/debug/forc build --help
forc-build
Compile the current or target project.
The output produced will depend on the project's program type.
- `script`, `predicate` and `contract` projects will produce their bytecode in binary format
`<project-name>.bin`.
- `script` projects will also produce a file containing the hash of the bytecode binary
`<project-name>-bin-hash` (using `fuel_cypto::Hasher`).
- `predicate` projects will also produce a file containing the **root** hash of the bytecode binary
`<project-name>-bin-root` (using `fuel_tx::Contract::root_from_code`).
- `contract` and `library` projects will also produce the public ABI in JSON format
`<project-name>-abi.json`.
USAGE:
forc build [OPTIONS]
OPTIONS:
--ast
Print the generated Sway AST (Abstract Syntax Tree)
--build-profile <BUILD_PROFILE>
Name of the build profile to use.
If unspecified, forc will use debug build profile.
--build-target <BUILD_TARGET>
Build target to use for code generation
[default: fuel]
[possible values: fuel, evm, midenvm]
--dca-graph <DCA_GRAPH>
Print the computed Sway DCA graph. DCA graph is printed to the specified path. If
specified '' graph is printed to stdout
--dca-graph-url-format <DCA_GRAPH_URL_FORMAT>
Specifies the url format to be used in the generated dot file.
Variables {path}, {line} {col} can be used in the provided format.
An example for vscode would be:
"vscode://file/{path}:{line}:{col}"
--error-on-warnings
Treat warnings as errors
--finalized-asm
Print the finalized ASM.
This is the state of the ASM with registers allocated and optimisations applied.
-g, --output-debug <DEBUG_FILE>
If set, outputs source file mapping in JSON format
-h, --help
Print help information
--intermediate-asm
Print the generated ASM.
This is the state of the ASM prior to performing register allocation and other ASM
optimisations.
--ipfs-node <IPFS_NODE>
The IPFS Node to use for fetching IPFS sources.
Possible values: PUBLIC, LOCAL, <GATEWAY_URL>
--ir
Print the generated Sway IR (Intermediate Representation)
--json-abi
By default the JSON for ABIs is formatted for human readability. By using this option
JSON output will be "minified", i.e. all on one line without whitespace
--json-abi-with-callpaths
Outputs json abi with callpaths instead of names for struct and enums
--json-storage-slots
By default the JSON for initial storage slots is formatted for human readability.
By using this option JSON output will be "minified", i.e. all on one line without
whitespace
-L, --log-level <LOG_LEVEL>
Set the log level
--locked
Requires that the Forc.lock file is up-to-date. If the lock file is missing, or it needs
to be updated, Forc will exit with an error
--metrics-outfile <METRICS_OUTFILE>
Output compilation metrics into file
-o, --output-bin <BIN_FILE>
If set, outputs a binary file representing the script bytes
--offline
Offline mode, prevents Forc from using the network when managing dependencies. Meaning
it will only try to use previously downloaded dependencies
--output-directory <OUTPUT_DIRECTORY>
The directory in which the sway compiler output artifacts are placed.
By default, this is `<project-root>/out`.
-p, --path <PATH>
Path to the project, if not specified, current working directory will be used
--release
Use release build plan. If a custom release plan is not specified, it is implicitly
added to the manifest file.
If --build-profile is also provided, forc omits this flag and uses provided
build-profile.
-s, --silent
Silence all output
-t, --terse
Terse mode. Limited warning and error output
--tests
Also build all tests within the project
--time-phases
Output the time elapsed over each part of the compilation process
-v, --verbose
Use verbose output |
|
Nice work Kaya, tested locally and it feels great. Happy to approve once CI is passing again. |
There was a problem hiding this comment.
Shameless self approve as @sdankel is OOO and we need a re-approval after merge conflicts fixed
kayagokalp
added
big
|
Also closes #4550 |
Description
Closes #3926.
(edited by @kayagokalp to reflect latest status of the PR)
This PR adds IPFS support for forc-pkg. To do so, forc relies on existing ipfs local node without embedding a node to forc itself. If local system does not have an ipfs node running, a public gateway (https://ipfs.io) is used as a fallback mechanism and packages are fetched through that.
Some Context and Future Work
For our package registry stuff to work properly we will eventually need to host an IPFS node to make sure that our published packages are pinned by at least one node in the network. That is the case as long as we don't have a way to incentivize people pinning the packages they published. What will happen is that a package will be published by a developer and after we detect the published package our own IPFS node will pin it to make sure it is always accessible.
Also some benchmarks are done for public gateway during development. It looks like for initial fetch operations from public gateway spends some time looking for the package pinned by the IPFS node simulating our incoming fuel IPFS node which is explained above. It is manageable but fetching from the fuel IPFS node is instant as it already got the package pinned. We can consider falling back to our own node's gateway api rather than a public one to smooth the process.
Finally maybe we can explore embedded ipfs node option as a follow-up, due to the status of IPFS with Rust, it is not very obvious (maybe not event possible atm) how to be able to be fully compatible with kubo, for fetching/publishing our packages. But this is still an open question worth exploring.
Testing
To test with a local node:
ipfs daemonservice withipfs daemonforc buildTo test public gateway fallback:
ipfs daemonor stop your local daemon withipfs shutdown.Add an ipfs source, I have a package already pinned by my IPFS node that you can use:
forc buildTODO:
pin,fetchanddep_pathimplementations.Checklist
Breaking*orNew Featurelabels where relevant.