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

ReadOnlyBlockstore in retrieval provider appears to track stores by deal id only, which is potentially non-unique #642

Open
hannahhoward opened this issue Oct 19, 2021 · 1 comment

Comments

@hannahhoward
Copy link
Collaborator

Hello:

I noticed we have:
https://github.com/filecoin-project/go-fil-markets/blob/master/retrievalmarket/impl/provider_environments.go#L138
and
https://github.com/filecoin-project/go-fil-markets/blob/master/retrievalmarket/impl/provider_environments.go#L261

It looks like you track your various DagShards in the retrieval provider by the deal only. While using a deal ID generated from time.Now() makes this less likely, deal ID is NOT gauranteed to be unique. For unique ID, you need to combine DealID + peer.ID -- which is why we use that combination for ProviderDealId, the value we index stored retrieval deals by elsewhere.

@dirkmc
Copy link
Contributor

dirkmc commented Oct 19, 2021

Agreed we need to fix it 👍
Thanks for the catch 🎣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants