From 05e0192563efa1bd1e0b33083d04deca5d463850 Mon Sep 17 00:00:00 2001 From: Arnaud Date: Tue, 6 May 2025 09:54:45 +0200 Subject: [PATCH] Returns collateral when a reservation is deleted and not only a slot is filled --- codex/sales/reservations.nim | 25 +++++++++++----------- codex/sales/states/cancelled.nim | 20 +++++++++-------- tests/codex/sales/states/testcancelled.nim | 5 +++-- tests/codex/sales/testsales.nim | 7 ++++++ 4 files changed, 34 insertions(+), 23 deletions(-) diff --git a/codex/sales/reservations.nim b/codex/sales/reservations.nim index fe4a82d3..7b1391ce 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,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" diff --git a/codex/sales/states/cancelled.nim b/codex/sales/states/cancelled.nim index f3c755a3..d3b4407c 100644 --- a/codex/sales/states/cancelled.nim +++ b/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) diff --git a/tests/codex/sales/states/testcancelled.nim b/tests/codex/sales/states/testcancelled.nim index ebd9fe2d..f9bc63e1 100644 --- a/tests/codex/sales/states/testcancelled.nim +++ b/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( 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)