refactor: filecoin client to use new capabilities #938
Conversation
There was a problem hiding this comment.
Everything looks good and as expected.
We just need to clarify the language for the invocations that will be used to basically issue receipts. I am also not sure on best way yet, but I think that at least it needs to mention that can also be a failure. Note that I only flagged first 2 occurrences, but it also happens in other cases
packages/filecoin-client/README.md
Outdated
|
|
||
| ### `Storefront.filecoinSubmit` | ||
|
|
||
| Signal that an offered piece has been submitted to the filecoin storage pipeline. |
There was a problem hiding this comment.
I think signal here is strange. I understand the perspective as its real purpose is to issue the receipt.
However, the receipt can actually be negative with an error instead of a signal that piece has been submitted.
I would probably rephrase to include something like: "Request to submit piece into the Filecoin Pipeline. Either will succeed and its receipt point to next task, or it will fail with reason to not have piece submitted in the pipeline (e.g. wrong piece provided for given content)"
Open to other suggestions, as this gets difficult to clairfy in a short format
There was a problem hiding this comment.
Maybe we want something more like Invoked to report whether piece was successfully submitted or not
There was a problem hiding this comment.
Note that I do not mind if we continue to use "signal" term as well. We just need to capture that it is invoked to report the asynchronous result of original invocation
packages/filecoin-client/README.md
Outdated
|
|
||
| ### `Storefront.filecoinAccept` | ||
|
|
||
| Signal that a submitted piece has been accepted in a Filecoin deal. |
There was a problem hiding this comment.
can also signal that it failed. same as above
| @@ -21,14 +19,15 @@ export const connection = connect({ | |||
| }) | |||
|
|
|||
| /** | |||
| * Queues a piece to the aggregator system of the filecoin pipeline. | |||
| * Request that a piece be aggregated for inclusion in an upcoming an Filecoin | |||
There was a problem hiding this comment.
| * Request that a piece be aggregated for inclusion in an upcoming an Filecoin | |
| * Request that a piece is aggregated for inclusion in an upcoming an Filecoin |
| }, | ||
| }) | ||
|
|
||
| // self invoke piece/add from aggregator | ||
| const res = await aggregateAdd( | ||
| // self invoke piece/offer from aggregator |
There was a problem hiding this comment.
| // self invoke piece/offer from aggregator | |
| // self invoke piece/accept from aggregator |
| assert.ok(!res.fx.join) | ||
| }) | ||
|
|
||
| it('aggregator self invokes add a filecoin piece to reject the piece queued', async () => { |
There was a problem hiding this comment.
nit: could we keep an error response?
docs: better docs fix: client tests
Implements `filecoin-api` services, events and tests all over the place. BREAKING CHANGE: see latest specs https://github.com/web3-storage/specs/blob/cbdb706f18567900c5c24d7fb16ccbaf93d0d023/w3-filecoin.md
vasco-santos
force-pushed
the
refactor/filecoin-client
branch
from
3bab254
to
c18abc8
Compare
vasco-santos
merged commit b80c8e6
into
refactor/filecoin-capabilities
Refactors the filecoin client to use the new capabilities.
TODO: fix up tests