filecoin-project / go-fil-markets

Shared Implementation of Storage and Retrieval Markets for Filecoin Node Implementations
Other
78 stars 59 forks source link

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

Closed dirkmc closed 3 years ago

dirkmc commented 3 years ago

This issue is likely the root cause of https://github.com/filecoin-project/lotus/issues/5829 and https://github.com/filecoin-project/lotus/issues/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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

Confirmed that this works on our miner 🎉