[marketplace] address more PR comments

- remove RequestState as no longer being used
- remove RequestState check on purchase timeout
- add tests for `withdrawFunds` and `subscribeRequestCancelled`
- update `dagger-contracts` so that `RequestCancelled.requestId` property is indexed in the event

codex/purchasing.nim

@@ -23,12 +23,6 @@ type
     clock: Clock
     request*: StorageRequest
   PurchaseTimeout* = Timeout
-  RequestState* = enum
-    New         = 1, # [default] waiting to fill slots
-    Started     = 2, # all slots filled, accepting regular proofs
-    Cancelled   = 3, # not enough slots filled before expiry
-    Finished    = 4, # successfully completed
-    Failed      = 5  # too many nodes have failed to provide proofs, data lost
   PurchaseId* = distinct array[32, byte]
 
 const DefaultProofProbability = 100.u256
@@ -81,7 +75,6 @@ func getPurchase*(purchasing: Purchasing, id: PurchaseId): ?Purchase =
 proc run(purchase: Purchase) {.async.} =
   let market = purchase.market
   let clock = purchase.clock
-  var state = RequestState.New
 
   proc requestStorage {.async.} =
     purchase.request = await market.requestStorage(purchase.request)
@@ -94,7 +87,6 @@ proc run(purchase: Purchase) {.async.} =
     let subscription = await market.subscribeFulfillment(request.id, callback)
     await done
     await subscription.unsubscribe()
-    state = RequestState.Started
 
   proc withTimeout(future: Future[void]) {.async.} =
     let expiry = purchase.request.expiry.truncate(int64)
@@ -104,15 +96,13 @@ proc run(purchase: Purchase) {.async.} =
   try:
     await waitUntilFulfilled().withTimeout()
   except PurchaseTimeout as e:
-    if state != RequestState.Started:
+    # If contract was fulfilled, the state would be RequestState.Started.
-      # If contract was fulfilled, the state would be RequestState.Started.
+    # Otherwise, the request would have timed out and should be considered
-      # Otherwise, the request would have timed out and should be considered
+    # cancelled. However, the request state hasn't been updated to
-      # cancelled. However, the request state hasn't been updated to
+    # RequestState.Cancelled yet so we can't check for that state or listen for
-      # RequestState.Cancelled yet so we can't check for that state or listen for
+    # an event emission. Instead, the state will be updated when the client
-      # an event emission. Instead, the state will be updated when the client
+    # requests to withdraw funds from the storage request.
-      # requests to withdraw funds from the storage request.
+    await market.withdrawFunds(purchase.request.id)
-      await market.withdrawFunds(purchase.request.id)
-      state = RequestState.Cancelled
     raise e
 
 proc start(purchase: Purchase) =
@@ -124,9 +114,6 @@ proc wait*(purchase: Purchase) {.async.} =
 func id*(purchase: Purchase): PurchaseId =
   PurchaseId(purchase.request.id)
 
-func cancelled*(purchase: Purchase): bool =
-  purchase.future.cancelled
-
 func finished*(purchase: Purchase): bool =
   purchase.future.finished
 

tests/codex/testpurchasing.nim

@@ -89,18 +89,25 @@ suite "Purchasing":
     expect PurchaseTimeout:
       await purchase.wait()
 
-  test "supports request cancelled subscription when request times out":
+  test "checks that funds were withdrawn when purchase times out":
     let purchase = purchasing.purchase(request)
     let request = market.requested[0]
     var receivedIds: seq[RequestId]
     clock.set(request.expiry.truncate(int64))
+
     proc onRequestCancelled(id: RequestId) {.gcsafe, upraises:[].} =
       receivedIds.add(id)
+
+    # will only be fired when `withdrawFunds` is called on purchase timeout
     let subscription = await market.subscribeRequestCancelled(
                                       request.id,
                                       onRequestCancelled)
+    var purchaseTimedOut = false
     try:
       await purchase.wait()
     except PurchaseTimeout:
-      check receivedIds == @[request.id]
+      purchaseTimedOut = true
-      await subscription.unsubscribe()
+
+    await subscription.unsubscribe()
+    check purchaseTimedOut
+    check receivedIds == @[request.id]

tests/contracts/testMarket.nim

@@ -1,6 +1,7 @@
 import pkg/chronos
 import codex/contracts
 import codex/contracts/testtoken
+import stew/byteutils # delete me
 import ../ethertest
 import ./examples
 import ./time
@@ -54,7 +55,14 @@ ethersuite "On-Chain Market":
     check (await market.getRequest(request.id)) == none StorageRequest
     await token.approve(storage.address, request.price)
     discard await market.requestStorage(request)
-    check (await market.getRequest(request.id)) == some request
+    let r = await market.getRequest(request.id)
+    check (r) == some request
+
+  test "supports withdrawing of funds":
+    await token.approve(storage.address, request.price)
+    discard await market.requestStorage(request)
+    await provider.advanceTimeTo(request.expiry)
+    await market.withdrawFunds(request.id)
 
   test "supports request subscriptions":
     var receivedIds: seq[RequestId]
@@ -144,3 +152,37 @@ ethersuite "On-Chain Market":
     check receivedIds == @[request.id]
 
     await subscription.unsubscribe()
+
+  test "support request cancelled subscriptions":
+    await token.approve(storage.address, request.price)
+    discard await market.requestStorage(request)
+
+    var receivedIds: seq[RequestId]
+    proc onRequestCancelled(id: RequestId) =
+      receivedIds.add(id)
+    let subscription = await market.subscribeRequestCancelled(request.id, onRequestCancelled)
+
+    await provider.advanceTimeTo(request.expiry)
+    await market.withdrawFunds(request.id)
+    check receivedIds == @[request.id]
+    await subscription.unsubscribe()
+
+  test "subscribes only to a certain request cancellation":
+    let otherRequest = StorageRequest.example
+    await token.approve(storage.address, request.price)
+    discard await market.requestStorage(request)
+    await token.approve(storage.address, otherRequest.price)
+    discard await market.requestStorage(otherRequest)
+
+    var receivedIds: seq[RequestId]
+    proc onRequestCancelled(requestId: RequestId) =
+      receivedIds.add(requestId)
+
+    let subscription = await market.subscribeRequestCancelled(request.id, onRequestCancelled)
+    await provider.advanceTimeTo(request.expiry) # shares expiry with otherRequest
+    expect ValueError:
+      await market.withdrawFunds(otherRequest.id)
+    check receivedIds.len == 0
+    await market.withdrawFunds(request.id)
+    check receivedIds == @[request.id]
+    await subscription.unsubscribe()

vendor/dagger-contracts

@@ -1 +1 @@
-Subproject commit 5c3aba160b8bd20dada45684d740727b3bf9048a
+Subproject commit 06bbfdb3a64428d1f5367f8cb6488ec093eddff5