Returns collateral when a reservation is deleted and not only a slot is filled

codex/sales/reservations.nim

@@ -392,6 +392,7 @@ proc deleteReservation*(
     availabilityId
 
   trace "deleting reservation"
+
   without key =? key(reservationId, availabilityId), error:
     return failure(error)
 
@@ -403,23 +404,23 @@ proc deleteReservation*(
         else:
           return failure(error)
 
+      without availabilityKey =? availabilityId.key, error:
+        return failure(error)
+
+      without var availability =? await self.get(availabilityKey, Availability), error:
+        return failure(error)
+
       if reservation.size > 0.uint64:
         trace "returning remaining reservation bytes to availability",
           size = reservation.size
-
-        without availabilityKey =? availabilityId.key, error:
-          return failure(error)
-
-        without var availability =? await self.get(availabilityKey, Availability), error:
-          return failure(error)
-
         availability.freeSize += reservation.size
 
-        if collateral =? returnedCollateral:
-          availability.totalRemainingCollateral += collateral
+      if collateral =? returnedCollateral:
+        availability.totalRemainingCollateral += collateral
+        trace "returning collateral", collateral = collateral
 
-        if updateErr =? (await self.updateAvailability(availability)).errorOption:
-          return failure(updateErr)
+      if updateErr =? (await self.updateAvailability(availability)).errorOption:
+        return failure(updateErr)
 
       if err =? (await self.repo.metaDs.ds.delete(key)).errorOption:
         return failure(err.toErr(DeleteFailedError))
@@ -510,7 +511,7 @@ method createReservation*(
       availability.totalRemainingCollateral -= slotSize.u256 * collateralPerByte
 
       # update availability with reduced size
-      trace "Updating availability with reduced size"
+      trace "Updating availability with reduced size", freeSize = availability.freeSize
       if updateErr =? (await self.updateAvailability(availability)).errorOption:
         trace "Updating availability failed, rolling back reservation creation"
 

codex/sales/states/cancelled.nim

@@ -31,22 +31,24 @@ method run*(
     raiseAssert "no sale request"
 
   try:
-    var returnedCollateral = UInt256.none
+    debug "Collecting collateral and partial payout",
+      requestId = data.requestId, slotIndex = data.slotIndex
+
+    # The returnedCollateral is needed even if the slot is not filled by the host
+    # because a reservation could be created and the collateral assigned
+    # to that reservation. So if the slot is not filled by that host,
+    # the reservation will be deleted during cleanup and the collateral
+    # must be returned to the host.
+    let slot = Slot(request: request, slotIndex: data.slotIndex)
+    let currentCollateral = await market.currentCollateral(slot.id)
+    let returnedCollateral = currentCollateral.some
 
     if await slotIsFilledByMe(market, data.requestId, data.slotIndex):
-      debug "Collecting collateral and partial payout",
-        requestId = data.requestId, slotIndex = data.slotIndex
-
-      let slot = Slot(request: request, slotIndex: data.slotIndex)
-      let currentCollateral = await market.currentCollateral(slot.id)
-
       try:
         await market.freeSlot(slot.id)
       except SlotStateMismatchError as e:
         warn "Failed to free slot because slot is already free", error = e.msg
 
-      returnedCollateral = currentCollateral.some
-
     if onClear =? agent.context.onClear and request =? data.request:
       onClear(request, data.slotIndex)
 

tests/codex/sales/states/testcancelled.nim

@@ -75,7 +75,8 @@ asyncchecksuite "sales state 'cancelled'":
     check eventually reprocessSlotWas == some false
     check eventually returnedCollateralValue == some currentCollateral
 
-  test "completes the cancelled state when free slot error is raised and the collateral is not returned when a host is not hosting a slot":
+  test "completes the cancelled state when free slot error is raised and the collateral is returned":
+    discard market.reserveSlot(requestId = request.id, slotIndex = slotIndex)
     market.fillSlot(
       requestId = request.id,
       slotIndex = slotIndex,
@@ -91,7 +92,7 @@ asyncchecksuite "sales state 'cancelled'":
     let next = await state.run(agent)
     check next == none State
     check eventually reprocessSlotWas == some false
-    check eventually returnedCollateralValue == UInt256.none
+    check eventually returnedCollateralValue == some currentCollateral
 
   test "calls onCleanUp and returns the collateral when an error is raised":
     market.fillSlot(

tests/codex/sales/testsales.nim

@@ -390,6 +390,13 @@ asyncchecksuite "Sales":
     await allowRequestToStart()
     await sold
 
+    # Disable the availability; otherwise, it will pick up the
+    # reservation again and we will not be able to check
+    # if the bytes are returned
+    availability.enabled = false
+    let result = await reservations.update(availability)
+    check result.isOk
+
     # complete request
     market.slotState[request.slotId(slotIndex)] = SlotState.Finished
     clock.advance(request.ask.duration.int64)