fix(slot-reservations): Avoid slot filled cancellations (#963)

* Avoid cancelling states when slot is filled

* improve logging

Improves logging for situations where a Sale should be ignored instead of being considered an error, including when reservation is not allowed and when a slot was filled by another host.

* remove onSlotFilled unit tests from states

codex/contracts/market.nim

@@ -165,8 +165,14 @@ method fillSlot(market: OnChainMarket,
                 proof: Groth16Proof,
                 collateral: UInt256) {.async.} =
   convertEthersError:
+    logScope:
+      requestId
+      slotIndex
+
     await market.approveFunds(collateral)
+    trace "calling fillSlot on contract"
     discard await market.contract.fillSlot(requestId, slotIndex, proof).confirm(0)
+    trace "fillSlot transaction completed"
 
 method freeSlot*(market: OnChainMarket, slotId: SlotId) {.async.} =
   convertEthersError:
@@ -253,7 +259,12 @@ method reserveSlot*(
   slotIndex: UInt256) {.async.} =
 
   convertEthersError:
-    discard await market.contract.reserveSlot(requestId, slotIndex).confirm(0)
+    discard await market.contract.reserveSlot(
+      requestId,
+      slotIndex,
+      # reserveSlot runs out of gas for unknown reason, but 100k gas covers it
+      TransactionOverrides(gasLimit: some 100000.u256)
+    ).confirm(0)
 
 method canReserveSlot*(
   market: OnChainMarket,

codex/sales/states/filling.nim

@@ -6,6 +6,8 @@ import ./errorhandling
 import ./filled
 import ./cancelled
 import ./failed
+import ./ignored
+import ./errored
 
 logScope:
   topics = "marketplace sales filling"
@@ -22,16 +24,25 @@ method onCancelled*(state: SaleFilling, request: StorageRequest): ?State =
 method onFailed*(state: SaleFilling, request: StorageRequest): ?State =
   return some State(SaleFailed())
 
-method onSlotFilled*(state: SaleFilling, requestId: RequestId,
-                     slotIndex: UInt256): ?State =
-  return some State(SaleFilled())
-
 method run(state: SaleFilling, machine: Machine): Future[?State] {.async.} =
   let data = SalesAgent(machine).data
   let market = SalesAgent(machine).context.market
   without (collateral =? data.request.?ask.?collateral):
     raiseAssert "Request not set"
 
-  debug "Filling slot", requestId = data.requestId, slotIndex = data.slotIndex
-  await market.fillSlot(data.requestId, data.slotIndex, state.proof, collateral)
-  debug "Waiting for slot filled event...", requestId = $data.requestId, slotIndex = $data.slotIndex
+  logScope:
+    requestId = data.requestId
+    slotIndex = data.slotIndex
+
+  debug "Filling slot"
+  try:
+    await market.fillSlot(data.requestId, data.slotIndex, state.proof, collateral)
+  except MarketError as e:
+    if e.msg.contains "Slot is not free":
+      debug "Slot is already filled, ignoring slot"
+      return some State( SaleIgnored(reprocessSlot: false, returnBytes: true) )
+    else:
+      return some State( SaleErrored(error: e) )
+  # other CatchableErrors are handled "automatically" by the ErrorHandlingState
+
+  return some State(SaleFilled())

codex/sales/states/slotreserving.nim

@@ -28,10 +28,6 @@ method onCancelled*(state: SaleSlotReserving, request: StorageRequest): ?State =
 method onFailed*(state: SaleSlotReserving, request: StorageRequest): ?State =
   return some State(SaleFailed())
 
-method onSlotFilled*(state: SaleSlotReserving, requestId: RequestId,
-                     slotIndex: UInt256): ?State =
-  return some State(SaleFilled())
-
 method run*(state: SaleSlotReserving, machine: Machine): Future[?State] {.async.} =
   let agent = SalesAgent(machine)
   let data = agent.data
@@ -48,7 +44,12 @@ method run*(state: SaleSlotReserving, machine: Machine): Future[?State] {.async.
       trace "Reserving slot"
       await market.reserveSlot(data.requestId, data.slotIndex)
     except MarketError as e:
-      return some State( SaleErrored(error: e) )
+      if e.msg.contains "Reservation not allowed":
+        debug "Slot cannot be reserved, ignoring", error = e.msg
+        return some State( SaleIgnored(reprocessSlot: false, returnBytes: true) )
+      else:
+        return some State( SaleErrored(error: e) )
+    # other CatchableErrors are handled "automatically" by the ErrorHandlingState
 
     trace "Slot successfully reserved"
     return some State( SaleDownloading() )

codex/utils/asyncstatemachine.nim

@@ -75,7 +75,7 @@ proc scheduler(machine: Machine) {.async.} =
           await running.cancelAndWait()
         let fromState = if machine.state.isNil: "<none>" else: $machine.state
         machine.state = next
-        debug "enter state", state = machine.state, fromState
+        debug "enter state", state = fromState & " => " & $machine.state
         running = machine.run(machine.state)
         running
           .track(machine)

tests/codex/sales/states/testfilling.nim

@@ -24,7 +24,3 @@ checksuite "sales state 'filling'":
   test "switches to failed state when request fails":
     let next = state.onFailed(request)
     check !next of SaleFailed
-
-  test "switches to filled state when slot is filled":
-    let next = state.onSlotFilled(request.id, slotIndex)
-    check !next of SaleFilled

tests/codex/sales/states/testslotreserving.nim

@@ -51,10 +51,6 @@ asyncchecksuite "sales state 'SlotReserving'":
     let next = state.onFailed(request)
     check !next of SaleFailed
 
-  test "switches to filled state when slot is filled":
-    let next = state.onSlotFilled(request.id, slotIndex)
-    check !next of SaleFilled
-
   test "run switches to downloading when slot successfully reserved":
     let next = await state.run(agent)
     check !next of SaleDownloading

tests/integration/hardhatprocess.nim

@@ -115,7 +115,7 @@ method onOutputLineCaptured(node: HardhatProcess, line: string) =
     return
 
   if error =? logFile.writeFile(line & "\n").errorOption:
-    error "failed to write to hardhat file", errorCode = error
+    error "failed to write to hardhat file", errorCode = $error
     discard logFile.closeFile()
     node.logFile = none IoHandle
 

tests/integration/nodeprocess.nim

@@ -128,7 +128,7 @@ method stop*(node: NodeProcess) {.base, async.} =
     try:
       trace "terminating node process..."
       if errCode =? node.process.terminate().errorOption:
-        error "failed to terminate process", errCode
+        error "failed to terminate process", errCode = $errCode
 
       trace "waiting for node process to exit"
       let exitCode = await node.process.waitForExit(3.seconds)