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

CID for /deals/content endpoint might be incorrect #24

Open
anjor opened this issue Feb 23, 2023 · 4 comments
Open

CID for /deals/content endpoint might be incorrect #24

anjor opened this issue Feb 23, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@anjor
Copy link
Contributor

anjor commented Feb 23, 2023

For users who have already calculated commP and are passing it in to /deals/content endpoint, the payload cid we are calculating could be incorrect depending on whether the file being passed through is a car file or not (side note: I wasn't quite able to figure out if we check that anywhere in the code)

Assuming it is a car file being passed through, then we are adding it to the local ipfs node and calculating the payload cid for the car file, as opposed to the original file.

At the end of the day I don't think it matters because payload cid is not really relevant for storage deals, but it does look like we are calculating it and passing it in the deal proposal.

@alvin-reyes
Copy link
Collaborator

alvin-reyes commented Feb 24, 2023

@anjor if you already have the commp (piece, padded size and size), you can add a metadata as part of the form request.

https://github.com/application-research/delta#upload-a-file

curl --location --request POST 'http://localhost:1414/api/v1/deal/content' \
--header 'Authorization: Bearer [ESTUARY_API_KEY]' \
--form 'data=@"/Users/alvinreyes/Downloads/baga6ea4seaqhfvwbdypebhffobtxjyp4gunwgwy2ydanlvbe6uizm5hlccxqmeq.car"' \
-form 'metadata="{\"miner\":\"f01963614\",
    \"piece_commitment\": {
        \"piece_cid\": \"baga6ea4seaqhfvwbdypebhffobtxjyp4gunwgwy2ydanlvbe6uizm5hlccxqmeq\",
        \"padded_piece_size\": 4294967296
    },
    \"connection_mode\": \"e2e\",
    \"size\": 2500366291
}"'

@anjor
Copy link
Contributor Author

anjor commented Feb 24, 2023

yeah that's fine. But in the code you still recalculate payload cid by pinning the uploaded car file:

pieceCommp.Cid = addNode.Cid().String()

which will now be the payload cid for the car file, and not for the original file.

@anjor anjor added the bug Something isn't working label Mar 1, 2023
@alvin-reyes alvin-reyes self-assigned this Mar 5, 2023
@alvin-reyes alvin-reyes added this to the feature-complete milestone Mar 15, 2023
@alvin-reyes
Copy link
Collaborator

the recalculation here is ok since we want to make sure that the payload cid complies with how delta prepare deals.

What we can do is add a new endpoint that accepts a pre-computed CID. I'll add to the next release.

@anjor
Copy link
Contributor Author

anjor commented Mar 26, 2023

Will this payload cid be reported back to the user? Because that might make for weird UX, where the user will get back a car instead of the data they expect.

@alvin-reyes alvin-reyes removed their assignment Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants