chore: returns the collateral when a slot is reserved but not filled (#1216)

* Change token allowance method because increaseAllowance does not exist anymore

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

* Remove the returnedCollateral when the slot is not filled by the host

* Add returnedCollateral when the sale is ignored

* Add returnsCollateral variable for ignored state

* Rebase the contracts submodule on the master

* Add integration test

* Fix duration

* Remove unnecessary teardown function

* Remove misleading comment

* Get returned collateral from the request

* Enable logs to debug on CI

* Fix test

* Increase test timeout

* Fix typo

* Fix rebase

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,22 @@ 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
 
-        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 +510,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/filling.nim

@@ -50,7 +50,7 @@ method run*(
       await market.fillSlot(data.requestId, data.slotIndex, state.proof, collateral)
     except SlotStateMismatchError as e:
       debug "Slot is already filled, ignoring slot"
-      return some State(SaleIgnored(reprocessSlot: false))
+      return some State(SaleIgnored(reprocessSlot: false, returnsCollateral: true))
     except MarketError as e:
       return some State(SaleErrored(error: e))
     # other CatchableErrors are handled "automatically" by the SaleState

codex/sales/states/ignored.nim

@@ -14,6 +14,7 @@ logScope:
 
 type SaleIgnored* = ref object of SaleState
   reprocessSlot*: bool # readd slot to queue with `seen` flag
+  returnsCollateral*: bool # returns collateral when a reservation was created
 
 method `$`*(state: SaleIgnored): string =
   "SaleIgnored"
@@ -22,10 +23,27 @@ method run*(
     state: SaleIgnored, machine: Machine
 ): Future[?State] {.async: (raises: []).} =
   let agent = SalesAgent(machine)
+  let data = agent.data
+  let market = agent.context.market
+
+  without request =? data.request:
+    raiseAssert "no sale request"
+
+  var returnedCollateral = UInt256.none
 
   try:
+    if state.returnsCollateral:
+      # The returnedCollateral is needed because a reservation could
+      # be created and the collateral assigned to that reservation.
+      # The returnedCollateral will be used in the cleanup function
+      # and be passed to the deleteReservation function.
+      let slot = Slot(request: request, slotIndex: data.slotIndex)
+      returnedCollateral = request.ask.collateralPerSlot.some
+
     if onCleanUp =? agent.onCleanUp:
-      await onCleanUp(reprocessSlot = state.reprocessSlot)
+      await onCleanUp(
+        reprocessSlot = state.reprocessSlot, returnedCollateral = returnedCollateral
+      )
   except CancelledError as e:
     trace "SaleIgnored.run was cancelled", error = e.msgDetail
   except CatchableError as e:

codex/sales/states/slotreserving.nim

@@ -46,7 +46,7 @@ method run*(
         await market.reserveSlot(data.requestId, data.slotIndex)
       except SlotReservationNotAllowedError as e:
         debug "Slot cannot be reserved, ignoring", error = e.msg
-        return some State(SaleIgnored(reprocessSlot: false))
+        return some State(SaleIgnored(reprocessSlot: false, returnsCollateral: true))
       except MarketError as e:
         return some State(SaleErrored(error: e))
       # other CatchableErrors are handled "automatically" by the SaleState
@@ -57,7 +57,7 @@ method run*(
       # do not re-add this slot to the queue, and return bytes from Reservation to
       # the Availability
       debug "Slot cannot be reserved, ignoring"
-      return some State(SaleIgnored(reprocessSlot: false))
+      return some State(SaleIgnored(reprocessSlot: false, returnsCollateral: true))
   except CancelledError as e:
     trace "SaleSlotReserving.run was cancelled", error = e.msgDetail
   except CatchableError as e:

tests/codex/sales/states/testcancelled.nim

@@ -76,6 +76,7 @@ asyncchecksuite "sales state 'cancelled'":
     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":
+    discard market.reserveSlot(requestId = request.id, slotIndex = slotIndex)
     market.fillSlot(
       requestId = request.id,
       slotIndex = slotIndex,

tests/codex/sales/states/testignored.nim

@@ -17,23 +17,34 @@ asyncchecksuite "sales state 'ignored'":
   let slotIndex = request.ask.slots div 2
   let market = MockMarket.new()
   let clock = MockClock.new()
+  let currentCollateral = UInt256.example
 
   var state: SaleIgnored
   var agent: SalesAgent
   var reprocessSlotWas = false
+  var returnedCollateralValue: ?UInt256
 
   setup:
     let onCleanUp = proc(
         reprocessSlot = false, returnedCollateral = UInt256.none
     ) {.async: (raises: []).} =
       reprocessSlotWas = reprocessSlot
+      returnedCollateralValue = returnedCollateral
 
     let context = SalesContext(market: market, clock: clock)
     agent = newSalesAgent(context, request.id, slotIndex, request.some)
     agent.onCleanUp = onCleanUp
     state = SaleIgnored.new()
+    returnedCollateralValue = UInt256.none
+    reprocessSlotWas = false
 
   test "calls onCleanUp with values assigned to SaleIgnored":
     state = SaleIgnored(reprocessSlot: true)
     discard await state.run(agent)
     check eventually reprocessSlotWas == true
+    check eventually returnedCollateralValue.isNone
+
+  test "returns collateral when returnsCollateral is true":
+    state = SaleIgnored(reprocessSlot: false, returnsCollateral: true)
+    discard await state.run(agent)
+    check eventually returnedCollateralValue.isSome

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)

tests/integration/testmarketplace.nim

@@ -316,3 +316,88 @@ marketplacesuite "Marketplace payouts":
     )
 
     await subscription.unsubscribe()
+
+  test "the collateral is returned after a sale is ignored",
+    NodeConfigs(
+      hardhat: HardhatConfig.none,
+      clients: CodexConfigs.init(nodes = 1).some,
+      providers: CodexConfigs.init(nodes = 3)
+      # .debug()
+      # uncomment to enable console log output
+      # .withLogFile()
+      # uncomment to output log file to tests/integration/logs/<start_datetime> <suite_name>/<test_name>/<node_role>_<node_idx>.log
+      # .withLogTopics(
+      #   "node", "marketplace", "sales", "reservations", "statemachine"
+      # )
+      .some,
+    ):
+    let data = await RandomChunker.example(blocks = blocks)
+    let client0 = clients()[0]
+    let provider0 = providers()[0]
+    let provider1 = providers()[1]
+    let provider2 = providers()[2]
+    let duration = 20 * 60.uint64
+    let slotSize = slotSize(blocks, ecNodes, ecTolerance)
+
+    # Here we create 3 SP which can host 3 slot.
+    # While they will process the slot, each SP will
+    # create a reservation for each slot.
+    # Likely we will have 1 slot by SP and the other reservations
+    # will be ignored. In that case, the collateral assigned for
+    # the reservation should return to the availability.
+    discard await provider0.client.postAvailability(
+      totalSize = 3 * slotSize.truncate(uint64),
+      duration = duration,
+      minPricePerBytePerSecond = minPricePerBytePerSecond,
+      totalCollateral = 3 * slotSize * minPricePerBytePerSecond,
+    )
+    discard await provider1.client.postAvailability(
+      totalSize = 3 * slotSize.truncate(uint64),
+      duration = duration,
+      minPricePerBytePerSecond = minPricePerBytePerSecond,
+      totalCollateral = 3 * slotSize * minPricePerBytePerSecond,
+    )
+    discard await provider2.client.postAvailability(
+      totalSize = 3 * slotSize.truncate(uint64),
+      duration = duration,
+      minPricePerBytePerSecond = minPricePerBytePerSecond,
+      totalCollateral = 3 * slotSize * minPricePerBytePerSecond,
+    )
+
+    let cid = (await client0.client.upload(data)).get
+
+    let purchaseId = await client0.client.requestStorage(
+      cid,
+      duration = duration,
+      pricePerBytePerSecond = minPricePerBytePerSecond,
+      proofProbability = 1.u256,
+      expiry = 10 * 60.uint64,
+      collateralPerByte = collateralPerByte,
+      nodes = ecNodes,
+      tolerance = ecTolerance,
+    )
+
+    let requestId = (await client0.client.requestId(purchaseId)).get
+
+    check eventually(
+      await client0.client.purchaseStateIs(purchaseId, "started"),
+      timeout = 10 * 60.int * 1000,
+    )
+
+    # Here we will check that for each provider, the total remaining collateral
+    # will match the available slots.
+    # So if a SP hosts 1 slot, it should have enough total remaining collateral
+    # to host 2 more slots.
+    for provider in providers():
+      let client = provider.client
+      check eventually(
+        block:
+          let availabilities = (await client.getAvailabilities()).get
+          let availability = availabilities[0]
+          let slots = (await client.getSlots()).get
+          let availableSlots = (3 - slots.len).u256
+
+          availability.totalRemainingCollateral ==
+            availableSlots * slotSize * minPricePerBytePerSecond,
+        timeout = 30 * 1000,
+      )