diff --git a/codex/sales/reservations.nim b/codex/sales/reservations.nim index fe4a82d3..d8ef2ae5 100644 --- a/codex/sales/reservations.nim +++ b/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" diff --git a/codex/sales/states/filling.nim b/codex/sales/states/filling.nim index 1b76150a..f0fcd4f3 100644 --- a/codex/sales/states/filling.nim +++ b/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 diff --git a/codex/sales/states/ignored.nim b/codex/sales/states/ignored.nim index 7f2ae5b1..ca0d48f7 100644 --- a/codex/sales/states/ignored.nim +++ b/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: diff --git a/codex/sales/states/slotreserving.nim b/codex/sales/states/slotreserving.nim index 780dadfc..4842f302 100644 --- a/codex/sales/states/slotreserving.nim +++ b/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: diff --git a/tests/codex/sales/states/testcancelled.nim b/tests/codex/sales/states/testcancelled.nim index ebd9fe2d..972051d4 100644 --- a/tests/codex/sales/states/testcancelled.nim +++ b/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, diff --git a/tests/codex/sales/states/testignored.nim b/tests/codex/sales/states/testignored.nim index b509ce9b..8b676387 100644 --- a/tests/codex/sales/states/testignored.nim +++ b/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 diff --git a/tests/codex/sales/testsales.nim b/tests/codex/sales/testsales.nim index 3a7a0750..f7c687f4 100644 --- a/tests/codex/sales/testsales.nim +++ b/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) diff --git a/tests/integration/testmarketplace.nim b/tests/integration/testmarketplace.nim index b2d44b3f..9b16f121 100644 --- a/tests/integration/testmarketplace.nim +++ b/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/ //_.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, + )