chore: add request validations (#1144)

* Add request validations

* Define expiry as required field in storage request params and fix tests

* Fix error messages

* Enable logs to figure out the issue with recurring failing test on macos

* Add custom errors raised by contract

* Remove custom error non existing anymore

* Update asynctest module

* Update timer tests after updating asynctest

codex/contracts/marketplace.nim

@@ -51,7 +51,6 @@ type
   Proofs_ProofNotMissing* = object of SolidityError
   Proofs_ProofNotRequired* = object of SolidityError
   Proofs_ProofAlreadyMarkedMissing* = object of SolidityError
-  Proofs_InvalidProbability* = object of SolidityError
   Periods_InvalidSecondsPerPeriod* = object of SolidityError
   SlotReservations_ReservationNotAllowed* = object of SolidityError
 
@@ -68,7 +67,9 @@ proc requestStorage*(
   errors: [
     Marketplace_InvalidClientAddress, Marketplace_RequestAlreadyExists,
     Marketplace_InvalidExpiry, Marketplace_InsufficientSlots,
-    Marketplace_InvalidMaxSlotLoss,
+    Marketplace_InvalidMaxSlotLoss, Marketplace_InsufficientDuration,
+    Marketplace_InsufficientProofProbability, Marketplace_InsufficientCollateral,
+    Marketplace_InsufficientReward, Marketplace_InvalidCid,
   ]
 .}
 

codex/rest/api.nim

@@ -652,10 +652,36 @@ proc initPurchasingApi(node: CodexNodeRef, router: var RestRouter) =
       without params =? StorageRequestParams.fromJson(body), error:
         return RestApiResponse.error(Http400, error.msg, headers = headers)
 
+      let expiry = params.expiry
+
+      if expiry <= 0 or expiry >= params.duration:
+        return RestApiResponse.error(
+          Http422,
+          "Expiry must be greater than zero and less than the request's duration",
+          headers = headers,
+        )
+
+      if params.proofProbability <= 0:
+        return RestApiResponse.error(
+          Http422, "Proof probability must be greater than zero", headers = headers
+        )
+
+      if params.collateralPerByte <= 0:
+        return RestApiResponse.error(
+          Http422, "Collateral per byte must be greater than zero", headers = headers
+        )
+
+      if params.pricePerBytePerSecond <= 0:
+        return RestApiResponse.error(
+          Http422,
+          "Price per byte per second must be greater than zero",
+          headers = headers,
+        )
+
       let requestDurationLimit = await contracts.purchasing.market.requestDurationLimit
       if params.duration > requestDurationLimit:
         return RestApiResponse.error(
-          Http400,
+          Http422,
           "Duration exceeds limit of " & $requestDurationLimit & " seconds",
           headers = headers,
         )
@@ -665,13 +691,13 @@ proc initPurchasingApi(node: CodexNodeRef, router: var RestRouter) =
 
       if tolerance == 0:
         return RestApiResponse.error(
-          Http400, "Tolerance needs to be bigger then zero", headers = headers
+          Http422, "Tolerance needs to be bigger then zero", headers = headers
         )
 
       # prevent underflow
       if tolerance > nodes:
         return RestApiResponse.error(
-          Http400,
+          Http422,
           "Invalid parameters: `tolerance` cannot be greater than `nodes`",
           headers = headers,
         )
@@ -682,21 +708,11 @@ proc initPurchasingApi(node: CodexNodeRef, router: var RestRouter) =
       # ensure leopard constrainst of 1 < K ≥ M
       if ecK <= 1 or ecK < ecM:
         return RestApiResponse.error(
-          Http400,
+          Http422,
           "Invalid parameters: parameters must satify `1 < (nodes - tolerance) ≥ tolerance`",
           headers = headers,
         )
 
-      without expiry =? params.expiry:
-        return RestApiResponse.error(Http400, "Expiry required", headers = headers)
-
-      if expiry <= 0 or expiry >= params.duration:
-        return RestApiResponse.error(
-          Http400,
-          "Expiry needs value bigger then zero and smaller then the request's duration",
-          headers = headers,
-        )
-
       without purchaseId =?
         await node.requestStorage(
           cid, params.duration, params.proofProbability, nodes, tolerance,
@@ -704,7 +720,7 @@ proc initPurchasingApi(node: CodexNodeRef, router: var RestRouter) =
         ), error:
         if error of InsufficientBlocksError:
           return RestApiResponse.error(
-            Http400,
+            Http422,
             "Dataset too small for erasure parameters, need at least " &
               $(ref InsufficientBlocksError)(error).minSize.int & " bytes",
             headers = headers,

codex/rest/json.nim

@@ -17,7 +17,7 @@ type
     proofProbability* {.serialize.}: UInt256
     pricePerBytePerSecond* {.serialize.}: UInt256
     collateralPerByte* {.serialize.}: UInt256
-    expiry* {.serialize.}: ?uint64
+    expiry* {.serialize.}: uint64
     nodes* {.serialize.}: ?uint
     tolerance* {.serialize.}: ?uint
 

openapi.yaml

@@ -800,6 +800,8 @@ paths:
                 type: string
         "400":
           description: Invalid or missing Request ID
+        "422":
+          description: The storage request parameters are not valid
         "404":
           description: Request ID not found
         "503":

tests/codex/utils/testtimer.nim

@@ -52,21 +52,21 @@ asyncchecksuite "Timer":
 
   test "Start timer1 should execute callback":
     startNumbersTimer()
-    check eventually output == "0"
+    check eventually(output == "0", pollInterval = 10)
 
   test "Start timer1 should execute callback multiple times":
     startNumbersTimer()
-    check eventually output == "012"
+    check eventually(output == "012", pollInterval = 10)
 
   test "Starting timer1 multiple times has no impact":
     startNumbersTimer()
     startNumbersTimer()
     startNumbersTimer()
-    check eventually output == "01234"
+    check eventually(output == "01234", pollInterval = 10)
 
   test "Stop timer1 should stop execution of the callback":
     startNumbersTimer()
-    check eventually output == "012"
+    check eventually(output == "012", pollInterval = 10)
     await timer1.stop()
     await sleepAsync(30.milliseconds)
     let stoppedOutput = output
@@ -81,4 +81,4 @@ asyncchecksuite "Timer":
   test "Starting both timers should execute callbacks sequentially":
     startNumbersTimer()
     startLettersTimer()
-    check eventually output == "0a1b2c3d4e"
+    check eventually(output == "0a1b2c3d4e", pollInterval = 10)

tests/integration/testpurchasing.nim

@@ -123,8 +123,9 @@ twonodessuite "Purchasing":
       proofProbability = 3.u256,
       collateralPerByte = 1.u256,
     )
-    check responseMissing.status == 400
+    check responseMissing.status == 422
-    check (await responseMissing.body) == "Expiry required"
+    check (await responseMissing.body) ==
+      "Expiry must be greater than zero and less than the request's duration"
 
     let responseBefore = await client1.requestStorageRaw(
       cid,
@@ -134,6 +135,6 @@ twonodessuite "Purchasing":
       collateralPerByte = 1.u256,
       expiry = 10.uint64,
     )
-    check responseBefore.status == 400
+    check responseBefore.status == 422
-    check "Expiry needs value bigger then zero and smaller then the request's duration" in
+    check "Expiry must be greater than zero and less than the request's duration" in
       (await responseBefore.body)

tests/integration/testrestapi.nim

@@ -69,7 +69,7 @@ twonodessuite "REST API":
     )
 
     check:
-      response.status == 400
+      response.status == 422
       (await response.body) ==
         "Dataset too small for erasure parameters, need at least " &
         $(2 * DefaultBlockSize.int) & " bytes"
@@ -109,7 +109,7 @@ twonodessuite "REST API":
       )
     )
 
-    check responseBefore.status == 400
+    check responseBefore.status == 422
     check (await responseBefore.body) == "Tolerance needs to be bigger then zero"
 
   test "request storage fails if duration exceeds limit", twoNodesConfig:
@@ -131,7 +131,7 @@ twonodessuite "REST API":
       )
     )
 
-    check responseBefore.status == 400
+    check responseBefore.status == 422
     check "Duration exceeds limit of" in (await responseBefore.body)
 
   test "request storage fails if nodes and tolerance aren't correct", twoNodesConfig:
@@ -154,7 +154,7 @@ twonodessuite "REST API":
         )
       )
 
-      check responseBefore.status == 400
+      check responseBefore.status == 422
       check (await responseBefore.body) ==
         "Invalid parameters: parameters must satify `1 < (nodes - tolerance) ≥ tolerance`"
 
@@ -179,10 +179,88 @@ twonodessuite "REST API":
         )
       )
 
-      check responseBefore.status == 400
+      check responseBefore.status == 422
       check (await responseBefore.body) ==
         "Invalid parameters: `tolerance` cannot be greater than `nodes`"
 
+  test "request storage fails if expiry is zero", twoNodesConfig:
+    let data = await RandomChunker.example(blocks = 2)
+    let cid = (await client1.upload(data)).get
+    let duration = 100.uint64
+    let pricePerBytePerSecond = 1.u256
+    let proofProbability = 3.u256
+    let expiry = 0.uint64
+    let collateralPerByte = 1.u256
+    let nodes = 3
+    let tolerance = 1
+
+    var responseBefore = await client1.requestStorageRaw(
+      cid, duration, pricePerBytePerSecond, proofProbability, collateralPerByte, expiry,
+      nodes.uint, tolerance.uint,
+    )
+
+    check responseBefore.status == 422
+    check (await responseBefore.body) ==
+      "Expiry must be greater than zero and less than the request's duration"
+
+  test "request storage fails if proof probability is zero", twoNodesConfig:
+    let data = await RandomChunker.example(blocks = 2)
+    let cid = (await client1.upload(data)).get
+    let duration = 100.uint64
+    let pricePerBytePerSecond = 1.u256
+    let proofProbability = 0.u256
+    let expiry = 30.uint64
+    let collateralPerByte = 1.u256
+    let nodes = 3
+    let tolerance = 1
+
+    var responseBefore = await client1.requestStorageRaw(
+      cid, duration, pricePerBytePerSecond, proofProbability, collateralPerByte, expiry,
+      nodes.uint, tolerance.uint,
+    )
+
+    check responseBefore.status == 422
+    check (await responseBefore.body) == "Proof probability must be greater than zero"
+
+  test "request storage fails if collareral per byte is zero", twoNodesConfig:
+    let data = await RandomChunker.example(blocks = 2)
+    let cid = (await client1.upload(data)).get
+    let duration = 100.uint64
+    let pricePerBytePerSecond = 1.u256
+    let proofProbability = 3.u256
+    let expiry = 30.uint64
+    let collateralPerByte = 0.u256
+    let nodes = 3
+    let tolerance = 1
+
+    var responseBefore = await client1.requestStorageRaw(
+      cid, duration, pricePerBytePerSecond, proofProbability, collateralPerByte, expiry,
+      nodes.uint, tolerance.uint,
+    )
+
+    check responseBefore.status == 422
+    check (await responseBefore.body) == "Collateral per byte must be greater than zero"
+
+  test "request storage fails if price per byte per second is zero", twoNodesConfig:
+    let data = await RandomChunker.example(blocks = 2)
+    let cid = (await client1.upload(data)).get
+    let duration = 100.uint64
+    let pricePerBytePerSecond = 0.u256
+    let proofProbability = 3.u256
+    let expiry = 30.uint64
+    let collateralPerByte = 1.u256
+    let nodes = 3
+    let tolerance = 1
+
+    var responseBefore = await client1.requestStorageRaw(
+      cid, duration, pricePerBytePerSecond, proofProbability, collateralPerByte, expiry,
+      nodes.uint, tolerance.uint,
+    )
+
+    check responseBefore.status == 422
+    check (await responseBefore.body) ==
+      "Price per byte per second must be greater than zero"
+
   for ecParams in @[
     (minBlocks: 2, nodes: 3, tolerance: 1), (minBlocks: 3, nodes: 5, tolerance: 2)
   ]:

tests/integration/testsales.nim

@@ -16,8 +16,18 @@ proc findItem[T](items: seq[T], item: T): ?!T =
 
 multinodesuite "Sales":
   let salesConfig = NodeConfigs(
-    clients: CodexConfigs.init(nodes = 1).some,
+    clients: CodexConfigs
-    providers: CodexConfigs.init(nodes = 1).some,
+      .init(nodes = 1)
+      .withLogFile()
+      .withLogTopics(
+        "node", "marketplace", "sales", "reservations", "node", "proving", "clock"
+      ).some,
+    providers: CodexConfigs
+      .init(nodes = 1)
+      .withLogFile()
+      .withLogTopics(
+        "node", "marketplace", "sales", "reservations", "node", "proving", "clock"
+      ).some,
   )
 
   let minPricePerBytePerSecond = 1.u256

vendor/asynctest

@@ -1 +1 @@
-Subproject commit 5154c0d79dd8bb086ab418cc659e923330ac24f2
+Subproject commit 73c08f77afc5cc2a5628d00f915b97bf72f70c9b