chore: address review comments

storacha · Nov 8, 2023 · 5bc4b2a · 5bc4b2a
1 parent 115237a
commit 5bc4b2a
Show file tree

Hide file tree

Showing 10 changed files with 28 additions and 253 deletions.
diff --git 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
@@ -48,17 +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'
 )

diff --git 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.Result<API.FilecoinInfoSuccess, API.FilecoinInfoFailure> | API.UcantoInterface.JoinBuilder<API.FilecoinInfoSuccess>>}
  */
 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<API.FilecoinInfoSuccess, API.FilecoinInfoFailure>} */
     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<API.FilecoinInfoSuccess, API.FilecoinInfoFailure>} */
   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
@@ -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
@@ -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<API.DealInfoSuccess, API.DealInfoFailure>} */
-                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)
-    },
 }
 
 /**