From 0157ca4c57eb05ab29b702213a63fbab8aa0c8d0 Mon Sep 17 00:00:00 2001 From: Eric <5089238+emizzle@users.noreply.github.com> Date: Thu, 24 Oct 2024 16:56:12 +1100 Subject: [PATCH] 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 | 13 +++++++++- codex/sales/states/filling.nim | 25 +++++++++++++------ codex/sales/states/slotreserving.nim | 11 ++++---- codex/utils/asyncstatemachine.nim | 2 +- tests/codex/sales/states/testfilling.nim | 4 --- .../codex/sales/states/testslotreserving.nim | 4 --- tests/integration/hardhatprocess.nim | 2 +- tests/integration/nodeprocess.nim | 2 +- 8 files changed, 39 insertions(+), 24 deletions(-) diff --git a/codex/contracts/market.nim b/codex/contracts/market.nim index 7cb639c8..6dd7e2b3 100644 --- a/codex/contracts/market.nim +++ b/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, diff --git a/codex/sales/states/filling.nim b/codex/sales/states/filling.nim index c96dd0b9..678e2a21 100644 --- a/codex/sales/states/filling.nim +++ b/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()) diff --git a/codex/sales/states/slotreserving.nim b/codex/sales/states/slotreserving.nim index d4343fd3..7e143173 100644 --- a/codex/sales/states/slotreserving.nim +++ b/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() ) diff --git a/codex/utils/asyncstatemachine.nim b/codex/utils/asyncstatemachine.nim index 9aeb3eab..3f10cfbe 100644 --- a/codex/utils/asyncstatemachine.nim +++ b/codex/utils/asyncstatemachine.nim @@ -75,7 +75,7 @@ proc scheduler(machine: Machine) {.async.} = await running.cancelAndWait() let fromState = if machine.state.isNil: "" 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) diff --git a/tests/codex/sales/states/testfilling.nim b/tests/codex/sales/states/testfilling.nim index ef2e2aa4..e68e2e70 100644 --- a/tests/codex/sales/states/testfilling.nim +++ b/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 diff --git a/tests/codex/sales/states/testslotreserving.nim b/tests/codex/sales/states/testslotreserving.nim index c54fb2aa..39419adc 100644 --- a/tests/codex/sales/states/testslotreserving.nim +++ b/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 diff --git a/tests/integration/hardhatprocess.nim b/tests/integration/hardhatprocess.nim index 6cfab47d..0e88aa78 100644 --- a/tests/integration/hardhatprocess.nim +++ b/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 diff --git a/tests/integration/nodeprocess.nim b/tests/integration/nodeprocess.nim index 97f4507f..61947c20 100644 --- a/tests/integration/nodeprocess.nim +++ b/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)