From 9f33fc424594f2c67bf11d821ebeb3514ad996a9 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Wed, 8 Nov 2023 16:47:59 +0100 Subject: [PATCH] chore: address review comments --- packages/capabilities/src/filecoin/lib.js | 6 +- .../capabilities/src/filecoin/storefront.js | 10 +- packages/filecoin-api/src/errors.js | 24 --- .../filecoin-api/src/storefront/service.js | 38 ++-- packages/filecoin-api/test/context/store.js | 8 +- .../filecoin-api/test/services/storefront.js | 187 ------------------ packages/filecoin-client/src/storefront.js | 6 +- .../filecoin-client/test/storefront.test.js | 8 +- packages/w3up-client/README.md | 1 - .../w3up-client/src/capability/filecoin.js | 7 +- .../test/capability/filecoin.test.js | 5 +- 11 files changed, 30 insertions(+), 270 deletions(-) diff --git a/packages/capabilities/src/filecoin/lib.js b/packages/capabilities/src/filecoin/lib.js index 76bc77271..fd4ae8cee 100644 --- a/packages/capabilities/src/filecoin/lib.js +++ b/packages/capabilities/src/filecoin/lib.js @@ -3,13 +3,11 @@ import { Schema } from '@ucanto/validator' /** * @see https://github.com/filecoin-project/FIPs/pull/758/files */ -export const FR32_SHA2_256_TRUNC254_PADDED_BINARY_TREE = /** @type {const} */ ( - 0x1011 -) +const FR32_SHA2_256_TRUNC254_PADDED_BINARY_TREE = /** @type {const} */ (0x1011) /** * @see https://github.com/filecoin-project/FIPs/pull/758/files */ -export const RAW_CODE = /** @type {const} */ (0x55) +const RAW_CODE = /** @type {const} */ (0x55) export const PieceLink = /** @type {import('../types.js').PieceLinkSchema} */ ( Schema.link({ diff --git a/packages/capabilities/src/filecoin/storefront.js b/packages/capabilities/src/filecoin/storefront.js index 0303a6692..53f0cc4b3 100644 --- a/packages/capabilities/src/filecoin/storefront.js +++ b/packages/capabilities/src/filecoin/storefront.js @@ -118,19 +118,17 @@ export const filecoinInfo = capability({ */ with: Schema.did(), nb: Schema.struct({ - /** - * CID of the content that resulted in Filecoin piece. - */ - content: Schema.link(), /** * CID of the piece. + * + * @see https://github.com/filecoin-project/FIPs/pull/758/files */ - piece: PieceLink.optional(), + piece: PieceLink, }), derives: (claim, from) => { return ( and(equalWith(claim, from)) || - and(checkLink(claim.nb.content, from.nb.content, 'nb.content')) || + and(checkLink(claim.nb.piece, from.nb.piece, 'nb.piece')) || ok({}) ) }, diff --git a/packages/filecoin-api/src/errors.js b/packages/filecoin-api/src/errors.js index df3a58db4..94f27277e 100644 --- a/packages/filecoin-api/src/errors.js +++ b/packages/filecoin-api/src/errors.js @@ -48,30 +48,6 @@ export class RecordNotFound extends Server.Failure { } } -export const ContentNotFoundErrorName = /** @type {const} */ ('ContentNotFound') -export class ContentNotFound extends Server.Failure { - get reason() { - return this.message - } - - get name() { - return ContentNotFoundErrorName - } -} - -export const InvalidContentPieceErrorName = /** @type {const} */ ( - 'InvalidContentPiece' -) -export class InvalidContentPiece extends Server.Failure { - get reason() { - return this.message - } - - get name() { - return InvalidContentPieceErrorName - } -} - export const EncodeRecordErrorName = /** @type {const} */ ('EncodeRecordFailed') export class EncodeRecordFailed extends Server.Failure { get reason() { diff --git a/packages/filecoin-api/src/storefront/service.js b/packages/filecoin-api/src/storefront/service.js index 0a2c27ba4..5f93a6177 100644 --- a/packages/filecoin-api/src/storefront/service.js +++ b/packages/filecoin-api/src/storefront/service.js @@ -9,8 +9,7 @@ import * as API from '../types.js' import { QueueOperationFailed, StoreOperationFailed, - ContentNotFound, - InvalidContentPiece, + RecordNotFoundErrorName, } from '../errors.js' /** @@ -238,26 +237,16 @@ async function findDataAggregationProof({ taskStore, receiptStore }, task) { * @returns {Promise | API.UcantoInterface.JoinBuilder>} */ export const filecoinInfo = async ({ capability }, context) => { - const { piece, content } = capability.nb + const { piece } = capability.nb - const queryRecords = await context.pieceStore.query({ content }) - if (queryRecords.error) { - return { error: new StoreOperationFailed(queryRecords.error.message) } - } else if (!queryRecords.ok.length) { + // Get piece in store + const getPiece = await context.pieceStore.get({ piece }) + if (getPiece.error && getPiece.error.name === RecordNotFoundErrorName) { return { - error: new ContentNotFound( - `no piece record was previously stored for content ${content.toString()}` - ), - } - } - if (Boolean(piece) && !queryRecords.ok[0].piece.equals(piece)) { - return { - error: new InvalidContentPiece( - `received piece ${piece?.toString()} is not the same as previously computed ${ - queryRecords.ok[0].piece - } for content ${content.toString()}` - ), + error: getPiece.error, } + } else if (getPiece.error) { + return { error: new StoreOperationFailed(getPiece.error.message) } } // Check if `piece/accept` receipt exists to get to know aggregate where it is included on a deal @@ -267,8 +256,8 @@ export const filecoinInfo = async ({ capability }, context) => { audience: context.id, with: context.id.did(), nb: { - piece: queryRecords.ok[0].piece, - content, + piece, + content: getPiece.ok.content, }, expiration: Infinity, }) @@ -278,10 +267,9 @@ export const filecoinInfo = async ({ capability }, context) => { pieceAcceptInvocation.link() ) if (pieceAcceptReceiptGet.error) { - // TODO: see receipt chain to report processing /** @type {API.UcantoInterface.OkBuilder} */ const processingResult = Server.ok({ - piece: queryRecords.ok[0].piece, + piece, deals: [], }) return processingResult @@ -308,14 +296,14 @@ export const filecoinInfo = async ({ capability }, context) => { // Should not happen if there is `piece/accept` receipt return { error: new Server.Failure( - `no deals were obtained for aggregate ${pieceAcceptOut.aggregate} where piece ${queryRecords.ok[0].piece} is included` + `no deals were obtained for aggregate ${pieceAcceptOut.aggregate} where piece ${piece} is included` ), } } /** @type {API.UcantoInterface.OkBuilder} */ const result = Server.ok({ - piece: queryRecords.ok[0].piece, + piece, deals: deals.map(([dealId, dealDetails]) => ({ aggregate: pieceAcceptOut.aggregate, provider: dealDetails.provider, diff --git a/packages/filecoin-api/test/context/store.js b/packages/filecoin-api/test/context/store.js index 51d19b1fa..02f98e61e 100644 --- a/packages/filecoin-api/test/context/store.js +++ b/packages/filecoin-api/test/context/store.js @@ -1,5 +1,5 @@ import * as API from '../../src/types.js' -import { RecordNotFound, StoreOperationFailed } from '../../src/errors.js' +import { StoreOperationFailed, RecordNotFound } from '../../src/errors.js' /** * @typedef {import('../../src/types.js').StorePutError} StorePutError @@ -47,7 +47,7 @@ export class Store { const t = this.getFn(this.items, item) if (!t) { return { - error: new RecordNotFound(), + error: new RecordNotFound('not found'), } } return { @@ -85,7 +85,7 @@ export class Store { const t = this.queryFn(this.items, search) if (!t) { return { - error: new RecordNotFound(), + error: new RecordNotFound('not found'), } } return { @@ -123,7 +123,7 @@ export class UpdatableStore extends Store { const t = this.updateFn(this.items, key, item) if (!t) { return { - error: new RecordNotFound(), + error: new RecordNotFound('not found'), } } return { diff --git a/packages/filecoin-api/test/services/storefront.js b/packages/filecoin-api/test/services/storefront.js index 347c2a711..fdc9c7c24 100644 --- a/packages/filecoin-api/test/services/storefront.js +++ b/packages/filecoin-api/test/services/storefront.js @@ -10,8 +10,6 @@ import * as StorefrontApi from '../../src/storefront/api.js' import { createServer, connect } from '../../src/storefront/service.js' import { - ContentNotFoundErrorName, - InvalidContentPieceErrorName, QueueOperationErrorName, StoreOperationErrorName, } from '../../src/errors.js' @@ -535,7 +533,6 @@ export const test = { with: agent.did(), nb: { piece: piece.link.link(), - content: piece.content.link(), }, }) @@ -587,149 +584,6 @@ export const test = { service ).connection - return { - ...context, - service, - dealTrackerService: { - connection: dealTrackerConnection, - invocationConfig: { - issuer: context.id, - with: context.id.did(), - audience: dealTrackerSigner, - }, - }, - } - } - ), - 'filecoin/info gets aggregate where piece was included together with deals and inclusion proof by giving content': - wichMockableContext( - async (assert, context) => { - const { agent, aggregator, dealer } = await getServiceContext() - const group = context.id.did() - const connection = connect({ - id: context.id, - channel: createServer(context), - }) - - // Create piece and aggregate for test - const { aggregate, pieces } = await randomAggregate(10, 128) - const piece = pieces[0] - const offer = pieces.map((p) => p.link) - const piecesBlock = await CBOR.write(offer) - - // Store piece into store - const putRes = await context.pieceStore.put({ - piece: piece.link.link(), - content: piece.content.link(), - group: context.id.did(), - status: 'submitted', - insertedAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }) - assert.ok(putRes.ok) - - // Create inclusion proof for test - const inclusionProof = aggregate.resolveProof(piece.link) - if (inclusionProof.error) { - throw new Error('could not compute inclusion proof') - } - - // Create invocations and receipts for chain into DealDataProof - const dealMetadata = { - dataType: 0n, - dataSource: { - dealID: 111n, - }, - } - const { invocations, receipts } = - await createInvocationsAndReceiptsForDealDataProofChain({ - storefront: context.id, - aggregator, - dealer, - aggregate: aggregate.link, - group, - piece: piece.link, - content: piece.content, - piecesBlock, - inclusionProof: { - subtree: inclusionProof.ok[0], - index: inclusionProof.ok[1], - }, - aggregateAcceptStatus: { - ...dealMetadata, - aggregate: aggregate.link, - }, - }) - - const storedInvocationsAndReceiptsRes = - await storeInvocationsAndReceipts({ - invocations, - receipts, - taskStore: context.taskStore, - receiptStore: context.receiptStore, - }) - assert.ok(storedInvocationsAndReceiptsRes.ok) - - // agent invocation - const filecoinInfoInv = Filecoin.info.invoke({ - issuer: agent, - audience: connection.id, - with: agent.did(), - nb: { - // Piece was previously set - piece: undefined, - content: piece.content.link(), - }, - }) - - const response = await filecoinInfoInv.execute(connection) - if (response.out.error) { - throw new Error('invocation failed', { cause: response.out.error }) - } - assert.ok(response.out.ok) - assert.ok(response.out.ok.piece.equals(piece.link.link())) - assert.equal(response.out.ok.deals.length, 1) - assert.ok(response.out.ok.deals[0].aggregate.equals(aggregate.link)) - assert.deepEqual( - BigInt(response.out.ok.deals[0].aux.dataType), - dealMetadata.dataType - ) - assert.deepEqual( - BigInt(response.out.ok.deals[0].aux.dataSource.dealID), - dealMetadata.dataSource.dealID - ) - assert.ok(response.out.ok.deals[0].inclusion.index) - assert.ok(response.out.ok.deals[0].inclusion.subtree) - }, - async (context) => { - /** - * Mock deal tracker to return deals - */ - const dealTrackerSigner = await Signer.generate() - const service = mockService({ - deal: { - info: Server.provideAdvanced({ - capability: DealTrackerCaps.dealInfo, - handler: async ({ invocation, context }) => { - /** @type {API.UcantoInterface.OkBuilder} */ - const result = Server.ok({ - deals: { - 111: { - provider: 'f11111', - }, - }, - }) - - return result - }, - }), - }, - }) - const dealTrackerConnection = getConnection( - dealTrackerSigner, - service - ).connection - return { ...context, service, @@ -762,53 +616,12 @@ export const test = { with: agent.did(), nb: { piece: piece.link, - content: piece.content.link(), }, }) const response = await filecoinInfoInv.execute(connection) assert.ok(response.out.error) - assert.equal(response.out.error?.name, ContentNotFoundErrorName) }, - 'filecoin/info fails if piece provided for content is different from the previously computed': - async (assert, context) => { - const { agent } = await getServiceContext() - const connection = connect({ - id: context.id, - channel: createServer(context), - }) - - // Create piece and aggregate for test - const { pieces } = await randomAggregate(10, 128) - const piece = pieces[0] - - // Store piece into store - const putRes = await context.pieceStore.put({ - piece: piece.link.link(), - content: piece.content.link(), - group: context.id.did(), - status: 'submitted', - insertedAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }) - assert.ok(putRes.ok) - - // agent invocation - const filecoinInfoInv = Filecoin.info.invoke({ - issuer: agent, - audience: connection.id, - with: agent.did(), - nb: { - // give wrong piece for content - piece: pieces[1].link, - content: piece.content.link(), - }, - }) - - const response = await filecoinInfoInv.execute(connection) - assert.ok(response.out.error) - assert.equal(response.out.error?.name, InvalidContentPieceErrorName) - }, } /** diff --git a/packages/filecoin-client/src/storefront.js b/packages/filecoin-client/src/storefront.js index 27be0f795..06cd36555 100644 --- a/packages/filecoin-client/src/storefront.js +++ b/packages/filecoin-client/src/storefront.js @@ -153,13 +153,11 @@ export async function filecoinAccept( * in Filecoin. It issues a signed receipt of the execution result. * * @param {import('./types.js').InvocationConfig} conf - Configuration - * @param {import('multiformats').UnknownLink} content - * @param {import('@web3-storage/data-segment').PieceLink} [piece] + * @param {import('@web3-storage/data-segment').PieceLink} piece * @param {import('./types.js').RequestOptions} [options] */ export async function filecoinInfo( { issuer, with: resource, proofs, audience }, - content, piece, options = {} ) { @@ -172,11 +170,9 @@ export async function filecoinInfo( audience: audience ?? services.STOREFRONT.principal, with: resource, nb: { - content, piece, }, proofs, - expiration: Infinity, }) return await invocation.execute(conn) diff --git a/packages/filecoin-client/test/storefront.test.js b/packages/filecoin-client/test/storefront.test.js index e490b49cd..557ed0507 100644 --- a/packages/filecoin-client/test/storefront.test.js +++ b/packages/filecoin-client/test/storefront.test.js @@ -284,12 +284,9 @@ describe('storefront', () => { assert.strictEqual(invCap.can, StorefrontCaps.filecoinInfo.can) assert.equal(invCap.with, invocation.issuer.did()) assert.ok(invCap.nb) - const { content, piece } = invCap.nb + const { piece } = invCap.nb // piece link - assert.ok(piece) - assert.ok(piece?.equals(cargo.link.link())) - // content - assert.ok(content.equals(cargo.content.link())) + assert.ok(piece.equals(cargo.link.link())) return Server.ok({ piece, @@ -306,7 +303,6 @@ describe('storefront', () => { with: agent.did(), audience: storefrontService, }, - cargo.content, cargo.link, { connection: getConnection(service).connection } ) diff --git a/packages/w3up-client/README.md b/packages/w3up-client/README.md index 86e51fcd3..699e60f8f 100644 --- a/packages/w3up-client/README.md +++ b/packages/w3up-client/README.md @@ -705,7 +705,6 @@ Offer a Filecoin "piece" to be added to an aggregate that will be offered for Fi ```ts function info ( - content: CID, piece: PieceLink ): Promise ``` diff --git a/packages/w3up-client/src/capability/filecoin.js b/packages/w3up-client/src/capability/filecoin.js index b7ab2b474..9a5c8e8d1 100644 --- a/packages/w3up-client/src/capability/filecoin.js +++ b/packages/w3up-client/src/capability/filecoin.js @@ -22,12 +22,11 @@ export class FilecoinClient extends Base { /** * Request info about a content piece in Filecoin deals * - * @param {import('multiformats').UnknownLink} content - * @param {import('@web3-storage/capabilities/types').PieceLink} [piece] + * @param {import('@web3-storage/capabilities/types').PieceLink} piece */ - async info(content, piece) { + async info(piece) { const conf = await this._invocationConfig([FilecoinCapabilities.info.can]) - return Storefront.filecoinInfo(conf, content, piece, { + return Storefront.filecoinInfo(conf, piece, { connection: this._serviceConf.filecoin, }) } diff --git a/packages/w3up-client/test/capability/filecoin.test.js b/packages/w3up-client/test/capability/filecoin.test.js index 1cf0d14ea..53427c4fc 100644 --- a/packages/w3up-client/test/capability/filecoin.test.js +++ b/packages/w3up-client/test/capability/filecoin.test.js @@ -143,10 +143,7 @@ describe('FilecoinClient', () => { alice.addSpace(auth) await alice.setCurrentSpace(space.did()) - const res = await alice.capability.filecoin.info( - cargo.content, - cargo.link - ) + const res = await alice.capability.filecoin.info(cargo.link) assert(service.filecoin.info.called) assert.equal(service.filecoin.info.callCount, 1)