Skip to content
This repository was archived by the owner on Jan 28, 2026. It is now read-only.

Changes the rpc contract to list cids metadata#13

Merged
brunocalza merged 2 commits intomainfrom
bcalza/listdealsfromhl
Nov 8, 2023
Merged

Changes the rpc contract to list cids metadata#13
brunocalza merged 2 commits intomainfrom
bcalza/listdealsfromhl

Conversation

@brunocalza
Copy link
Contributor

@brunocalza brunocalza commented Oct 31, 2023

Summary

Implements the changes necessary to work with tablelandnetwork/basin-provider#12

Implementation

The implementation is very straight forward:

  • It changes the capnp definitions
  • And adapts the code to return those new info

Signed-off-by: Bruno Calza <brunoangelicalza@gmail.com>
Name: "latest",
Usage: "The latest N deals to fetch",
Destination: &latest,
Value: 10,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the default value for latest because of this IF . if latest has a default value, limit and offset will never be used

@brunocalza brunocalza self-assigned this Oct 31, 2023
@brunocalza brunocalza marked this pull request as ready for review November 1, 2023 13:12
// DealInfo represents information about a deal.
type DealInfo struct {
ID uint64
SelectorPath string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so there won't any way to know the selector path for the data?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't see a reason for having that. but we could add if that makes sense

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a deal could have many uploaded files in it, correct? ie, user uploads files a, b, c. web3.storage gives us back a cid for each, but may put them all in one, two, or three deals... we don't know. so in order to get the data for cid a out of whatever deal it's in, we need a way to identify it in the CAR file. or am i complicating it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, thats my understanding too based on this doc.

i think, for hot layer retrieval selectorPath is not required because we have CIDs for each file that we upload. but if the data is purged from hot storage, then the only way to get it would be from storage provider, right? which would require a selector path.

i also added a comment on the other PR but it is related: tablelandnetwork/basin-provider#12 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum, yeah i get that. I was kind of with lassie in my mind, with it, you can retrieve only with the CID, e.g.

lassie fetch bafybeifmmy2wvmx4bvizt5nwwccktewyjjrjskngmt2dn77p5yklucgg4i

Not sure what kind of magic is going on there, but looks like it's definitely possible to retrieve from Filecoin only with CID.

I can add the selector path back, but looks like we'd need a deal id as well, right? Because the ID that I removed is the ID of the database. And, then you have the problem of a file being in multiple deals, so you have to actually return multiple deal ids

Copy link
Contributor

@avichalp avichalp Nov 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what kind of magic is going on there, but looks like it's definitely possible to retrieve from Filecoin only with CID.

interesting! we should test it somehow but I thought lassie is only able to fetch data using just the CID because w3s is pinning it. and it will continue pinning until we keeping paying them. this is why i am not sure what happens to the CID if it not pinned by w3s anymore

And, then you have the problem of a file being in multiple deals, so you have to actually return multiple deal ids

yup, w3s usually puts in the input file in at least two separate deals. not sure how this info can be presented to the user

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point! i thought lassie was only a Filecoin retriever, but could be the case that they are using IPFS

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asked on Filecion slack channel

  • Does lassie fetches content from some Filecoin storage provider or from the IPFS node that pinned the file?

Both

  • How can I make sure lassie fetches only from Filecoin?

You can pass a flag to lassie to limit either the protocols or providers to allow

  • And how from only the CID lassie can figure out the deal, and the path inside the deal where the content is?

lassie looks up the the CID in #ipni - the network indexers - which store a database of CID->provider.
For filecoin providers, the provider locally stores metadata to help it serve the CIDs it advertises - like where they are inside the deal

So, looks like with only the CID the client can retrieve a file, we can make use of lassie as a library if needed.

Also, if we want to list all deals of a file, we can create another command specific to that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is great, so makes sense to remove the path and ID 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet

Signed-off-by: Bruno Calza <brunoangelicalza@gmail.com>
Copy link
Contributor

@avichalp avichalp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@brunocalza brunocalza merged commit 8ee5203 into main Nov 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants