From adfb56f3539af440cc25d24aaa4f8ed0611d1d1f Mon Sep 17 00:00:00 2001
From: Vasco Santos <santos.vasco10@gmail.com>
Date: Fri, 12 Apr 2024 09:56:00 +0200
Subject: [PATCH] chore: address last review comments

---
 packages/upload-api/src/blob/add.js      | 10 ++++++----
 packages/upload-api/src/blob/allocate.js |  3 +++
 packages/upload-api/src/ucan/conclude.js |  2 ++
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/packages/upload-api/src/blob/add.js b/packages/upload-api/src/blob/add.js
index 17896ab67..0d0fdc533 100644
--- a/packages/upload-api/src/blob/add.js
+++ b/packages/upload-api/src/blob/add.js
@@ -159,7 +159,7 @@ async function allocate({ context, blob, space, cause }) {
     }
   }
 
-  // 3. if not already allocated (or expired) schedule `blob/allocate`
+  // 3. if not already allocated (or expired) execute `blob/allocate`
   if (!blobAllocateReceipt) {
     // Execute allocate invocation
     const allocateRes = await allocate.execute(context.getServiceConnection())
@@ -208,7 +208,7 @@ async function put({ context, blob, allocateTask }) {
   // could perform the invocation and issue receipt by deriving same
   // principal
   const blobProvider = await ed25519.derive(
-    blob.digest.slice(blob.digest.length - 32)
+    blob.digest.subarray(-32)
   )
   const facts = [
     {
@@ -235,6 +235,8 @@ async function put({ context, blob, allocateTask }) {
 
   // 2. Get receipt for `http/put` if available
   const receiptGet = await context.receiptsStorage.get(task.link())
+  // Storage get can fail with `RecordNotFound` or other unexpected errors.
+  // If 'RecordNotFound' we proceed, otherwise we fail with the received error.
   if (receiptGet.error && receiptGet.error.name !== 'RecordNotFound') {
     return {
       error: receiptGet.error,
@@ -306,7 +308,7 @@ async function accept({ context, blob, space, putTask, putReceipt }) {
     }
   }
 
-  // 3. Get receipt for `blob/accept` if available, otherwise schedule invocation
+  // 3. Get receipt for `blob/accept` if available, otherwise execute invocation
   let blobAcceptReceipt
   const receiptGet = await context.receiptsStorage.get(task.link())
   if (receiptGet.error && receiptGet.error.name !== 'RecordNotFound') {
@@ -317,7 +319,7 @@ async function accept({ context, blob, space, putTask, putReceipt }) {
     blobAcceptReceipt = receiptGet.ok
   }
 
-  // 4. if not already accepted schedule `blob/accept`
+  // 4. if not already accepted execute `blob/accept`
   if (!blobAcceptReceipt) {
     // Execute accept invocation
     const acceptRes = await accept.execute(context.getServiceConnection())
diff --git a/packages/upload-api/src/blob/allocate.js b/packages/upload-api/src/blob/allocate.js
index 7abd151a7..a1f256040 100644
--- a/packages/upload-api/src/blob/allocate.js
+++ b/packages/upload-api/src/blob/allocate.js
@@ -51,12 +51,15 @@ export function blobAllocateProvider(context) {
       }
 
       // Check if blob already exists
+      // TODO: this may depend on the region we want to allocate and will need changes in the future.
       const hasBlobStore = await context.blobsStorage.has(blob.digest)
       if (hasBlobStore.error) {
         return hasBlobStore
       }
 
       // If blob is stored, we can just allocate it to the space with the allocated size
+      // TODO: this code path MAY lead to await failures - awaited http/put and blob/accept tasks
+      // are supposed to fail if path does not exists.
       if (hasBlobStore.ok) {
         return {
           ok: { size },
diff --git a/packages/upload-api/src/ucan/conclude.js b/packages/upload-api/src/ucan/conclude.js
index dcc49cb2d..564811efe 100644
--- a/packages/upload-api/src/ucan/conclude.js
+++ b/packages/upload-api/src/ucan/conclude.js
@@ -82,6 +82,8 @@ export const ucanConcludeProvider = ({
                   'ucan/await': ['.out.ok', ranInvocation.link()],
                 },
               },
+              // Expiry is set to `Infinity` so that CID will come out the same
+              // as returned in effect on `blob/add`
               expiration: Infinity,
             })
             .delegate()