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

When retrieval unsealing price > 0, provider tries to start sending blocks before unsealing is complete #528

Closed
dirkmc opened this issue Mar 31, 2021 · 4 comments · Fixed by #525
Assignees
Labels
x/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs

Comments

@dirkmc
Copy link
Contributor

dirkmc commented Mar 31, 2021

This issue is likely the root cause of filecoin-project/lotus#5829 and filecoin-project/lotus#5530

Background

When the retrieval unseal price is > 0, the sequence should be:

  1. Client opens channel to provider
  2. Provider requests unseal payment
  3. Client sends unseal payment
  4. If Provider dosen't have an unsealed copy lying around, it unseals the Sector. If it has an unsealed copy lying around, this step is a no-op.
  5. Provider stores unsealed data in blockstore
  6. Provider sends blocks to client

However there is a bug whereby currently the provider starts trying to send blocks as soon as it receives the unseal payment. It does not wait until the unsealed data has been read into the blockstore. The result is that graphsync fails to read the expected blocks from the blockstore and the deal fails.

Proposed solution

The ProviderRevalidator should return ErrPause from Revalidate() if the unsealed deal data hasn't been read into the blockstore.

@kernelogic
Copy link

kernelogic commented Mar 31, 2021

But why does it want unseal payment when the deal is fast-retrieval (already unsealed)? Is that how it supposed to work, always unseal regardless it's fast-retrieval or not?

@aarshkshah1992
Copy link
Collaborator

aarshkshah1992 commented Apr 1, 2021

@kernelogic

It's not that we unseal even if we already have an unsealed copy lying around.

It's that we resume the Graphsync transfer before we've read the unsealed data into the Blockstore that Graphsync reads from.

I've updated the issue to reflect this.

@kernelogic
Copy link

@aarshkshah1992 I apologize if my previous comment was not clear.

From my observation, right now I always have to pay unsealing price when retrieving a deal I made, even I made it with fast-retrieval=true.

From my understanding I shouldn't have to pay the unsealing price if it's fast-retrieval, right?

@dirkmc
Copy link
Contributor Author

dirkmc commented Apr 2, 2021

Confirmed that this works on our miner 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants