Fix concurrency issues (#993)

* Use http subscriptions instead of websocket for tests

To work around this issue when subscriptions are
inactive for more than 5 minutes:
NomicFoundation/hardhat#2053

Use 100 millisecond polling; default polling interval
of 4 seconds is too close to the 5 second timeout for
`check eventually`.

* use .confirm(1) instead of confirm(0)

confirm(0) doesn't wait at all, confirm(1) waits
for the transaction to be mined

* speed up partial payout integration test

* update nim-ethers to version 0.10.0

includes fixes for http polling and .confirm()

* fix timing of marketplace tests

allow for a bit more time to withdraw funds

* use .confirm(1) in marketplace tests

to ensure that the transaction has been processed
before continuing with the test

* fix timing issue in validation unit test

* fix proof integration test

there were two logic errors in this test:
- a slot is freed anyway at the end of the contract
- when starting the request takes a long time, the
  first slot can already be freed because there were
  too many missing proofs

* fix intermittent error in contract tests

currentTime() doesn't always correctly reflect
the time of the next transaction

* reduce number of slots in integration test

otherwise the windows runner in the CI won't
be able to start the request before it expires

* fix timing in purchasing test

allow for a bit more time for a request to
be submitted

* fix timing of request submission in test

windows ci is so slow, it can take up to 40 seconds
just to submit a storage request to hardhat

* increase proof period to 90 seconds

* adjust timing of integration tests

reason: with the increased period length of 90 seconds, it
can take longer to wait for a stable challenge at the
beginning of a period.

* increase CI timeout to 2 hours

* Fix slow builds on windows

apparently it takes windows 2-3 seconds to
resolve "localhost" to 127.0.0.1 for every
json-rpc connection that we make 🤦

.github/workflows/ci-reusable.yml

@@ -26,7 +26,7 @@ jobs:
 
     name: ${{ matrix.os }}-${{ matrix.tests }}-${{ matrix.cpu }}-${{ matrix.nim_version }}
     runs-on: ${{ matrix.builder }}
-    timeout-minutes: 100
+    timeout-minutes: 120
     steps:
       - name: Checkout sources
         uses: actions/checkout@v4

codex/contracts/market.nim

@@ -53,7 +53,7 @@ proc approveFunds(market: OnChainMarket, amount: UInt256) {.async.} =
   convertEthersError:
     let tokenAddress = await market.contract.token()
     let token = Erc20Token.new(tokenAddress, market.signer)
-    discard await token.increaseAllowance(market.contract.address(), amount).confirm(0)
+    discard await token.increaseAllowance(market.contract.address(), amount).confirm(1)
 
 method getZkeyHash*(market: OnChainMarket): Future[?string] {.async.} =
   let config = await market.contract.configuration()
@@ -99,7 +99,7 @@ method requestStorage(market: OnChainMarket, request: StorageRequest){.async.} =
   convertEthersError:
     debug "Requesting storage"
     await market.approveFunds(request.price())
-    discard await market.contract.requestStorage(request).confirm(0)
+    discard await market.contract.requestStorage(request).confirm(1)
 
 method getRequest(market: OnChainMarket,
                   id: RequestId): Future[?StorageRequest] {.async.} =
@@ -171,7 +171,7 @@ method fillSlot(market: OnChainMarket,
 
     await market.approveFunds(collateral)
     trace "calling fillSlot on contract"
-    discard await market.contract.fillSlot(requestId, slotIndex, proof).confirm(0)
+    discard await market.contract.fillSlot(requestId, slotIndex, proof).confirm(1)
     trace "fillSlot transaction completed"
 
 method freeSlot*(market: OnChainMarket, slotId: SlotId) {.async.} =
@@ -191,13 +191,13 @@ method freeSlot*(market: OnChainMarket, slotId: SlotId) {.async.} =
       # recipient (the contract will use msg.sender for both)
       freeSlot = market.contract.freeSlot(slotId)
 
-    discard await freeSlot.confirm(0)
+    discard await freeSlot.confirm(1)
 
 
 method withdrawFunds(market: OnChainMarket,
                      requestId: RequestId) {.async.} =
   convertEthersError:
-    discard await market.contract.withdrawFunds(requestId).confirm(0)
+    discard await market.contract.withdrawFunds(requestId).confirm(1)
 
 method isProofRequired*(market: OnChainMarket,
                         id: SlotId): Future[bool] {.async.} =
@@ -230,13 +230,13 @@ method submitProof*(market: OnChainMarket,
                     id: SlotId,
                     proof: Groth16Proof) {.async.} =
   convertEthersError:
-    discard await market.contract.submitProof(id, proof).confirm(0)
+    discard await market.contract.submitProof(id, proof).confirm(1)
 
 method markProofAsMissing*(market: OnChainMarket,
                            id: SlotId,
                            period: Period) {.async.} =
   convertEthersError:
-    discard await market.contract.markProofAsMissing(id, period).confirm(0)
+    discard await market.contract.markProofAsMissing(id, period).confirm(1)
 
 method canProofBeMarkedAsMissing*(
     market: OnChainMarket,
@@ -264,7 +264,7 @@ method reserveSlot*(
       slotIndex,
       # reserveSlot runs out of gas for unknown reason, but 100k gas covers it
       TransactionOverrides(gasLimit: some 100000.u256)
-    ).confirm(0)
+    ).confirm(1)
 
 method canReserveSlot*(
   market: OnChainMarket,

tests/codex/testvalidation.nim

@@ -67,15 +67,15 @@ asyncchecksuite "validation":
       check validationConfig.error.msg == "The value of the group index " &
           "must be less than validation groups! " &
           fmt"(got: {groupIndex = }, groups = {!groups})"
-  
+
   test "initializing ValidationConfig fails when maxSlots is negative":
     let maxSlots = -1
     let validationConfig = ValidationConfig.init(
         maxSlots = maxSlots, groups = ValidationGroups.none)
     check validationConfig.isFailure == true
     check validationConfig.error.msg == "The value of maxSlots must " &
         fmt"be greater than or equal to 0! (got: {maxSlots})"
-  
+
   test "initializing ValidationConfig fails when maxSlots is negative " &
       "(validationGroups set)":
     let maxSlots = -1
@@ -97,7 +97,7 @@ asyncchecksuite "validation":
   test "when a slot is filled on chain, it is added to the list":
     await market.fillSlot(slot.request.id, slot.slotIndex, proof, collateral)
     check validation.slots == @[slot.id]
-  
+
   test "slot should be observed if maxSlots is set to 0":
     let validationConfig = initValidationConfig(
         maxSlots = 0, ValidationGroups.none)
@@ -128,14 +128,14 @@ asyncchecksuite "validation":
     await market.fillSlot(slot.request.id, slot.slotIndex, proof, collateral)
     market.setCanProofBeMarkedAsMissing(slot.id, true)
     advanceToNextPeriod()
-    await sleepAsync(1.millis)
+    await sleepAsync(100.millis) # allow validation loop to run
     check market.markedAsMissingProofs.contains(slot.id)
 
   test "when a proof can not be marked as missing, it will not be marked":
     await market.fillSlot(slot.request.id, slot.slotIndex, proof, collateral)
     market.setCanProofBeMarkedAsMissing(slot.id, false)
     advanceToNextPeriod()
-    await sleepAsync(1.millis)
+    await sleepAsync(100.millis) # allow validation loop to run
     check market.markedAsMissingProofs.len == 0
 
   test "it does not monitor more than the maximum number of slots":

tests/contracts/testClock.nim

@@ -30,7 +30,7 @@ ethersuite "On-Chain Clock":
     let waiting = clock.waitUntil(future)
     discard await ethProvider.send("evm_setNextBlockTimestamp", @[%future])
     discard await ethProvider.send("evm_mine")
-    check await waiting.withTimeout(chronos.milliseconds(100))
+    check await waiting.withTimeout(chronos.milliseconds(500))
 
   test "can wait until a certain time is reached by the wall-clock":
     let future = clock.now() + 1 # seconds

tests/contracts/testContracts.nim

@@ -45,13 +45,13 @@ ethersuite "Marketplace contracts":
     request.client = await client.getAddress()
 
     switchAccount(client)
-    discard await token.approve(marketplace.address, request.price)
-    discard await marketplace.requestStorage(request)
+    discard await token.approve(marketplace.address, request.price).confirm(1)
+    discard await marketplace.requestStorage(request).confirm(1)
     switchAccount(host)
-    discard await token.approve(marketplace.address, request.ask.collateral)
-    discard await marketplace.reserveSlot(request.id, 0.u256)
-    filledAt = await ethProvider.currentTime()
-    discard await marketplace.fillSlot(request.id, 0.u256, proof)
+    discard await token.approve(marketplace.address, request.ask.collateral).confirm(1)
+    discard await marketplace.reserveSlot(request.id, 0.u256).confirm(1)
+    let receipt = await marketplace.fillSlot(request.id, 0.u256, proof).confirm(1)
+    filledAt = await ethProvider.blockTime(BlockTag.init(!receipt.blockNumber))
     slotId = request.slotId(0.u256)
 
   proc waitUntilProofRequired(slotId: SlotId) {.async.} =
@@ -65,14 +65,14 @@ ethersuite "Marketplace contracts":
 
   proc startContract() {.async.} =
     for slotIndex in 1..<request.ask.slots:
-      discard await token.approve(marketplace.address, request.ask.collateral)
-      discard await marketplace.reserveSlot(request.id, slotIndex.u256)
-      discard await marketplace.fillSlot(request.id, slotIndex.u256, proof)
+      discard await token.approve(marketplace.address, request.ask.collateral).confirm(1)
+      discard await marketplace.reserveSlot(request.id, slotIndex.u256).confirm(1)
+      discard await marketplace.fillSlot(request.id, slotIndex.u256, proof).confirm(1)
 
   test "accept marketplace proofs":
     switchAccount(host)
     await waitUntilProofRequired(slotId)
-    discard await marketplace.submitProof(slotId, proof)
+    discard await marketplace.submitProof(slotId, proof).confirm(1)
 
   test "can mark missing proofs":
     switchAccount(host)
@@ -81,7 +81,7 @@ ethersuite "Marketplace contracts":
     let endOfPeriod = periodicity.periodEnd(missingPeriod)
     await ethProvider.advanceTimeTo(endOfPeriod + 1)
     switchAccount(client)
-    discard await marketplace.markProofAsMissing(slotId, missingPeriod)
+    discard await marketplace.markProofAsMissing(slotId, missingPeriod).confirm(1)
 
   test "can be paid out at the end":
     switchAccount(host)
@@ -90,7 +90,7 @@ ethersuite "Marketplace contracts":
     let requestEnd = await marketplace.requestEnd(request.id)
     await ethProvider.advanceTimeTo(requestEnd.u256 + 1)
     let startBalance = await token.balanceOf(address)
-    discard await marketplace.freeSlot(slotId)
+    discard await marketplace.freeSlot(slotId).confirm(1)
     let endBalance = await token.balanceOf(address)
     check endBalance == (startBalance + expectedPayout(requestEnd.u256) + request.ask.collateral)
 
@@ -103,7 +103,7 @@ ethersuite "Marketplace contracts":
     let startBalanceHost = await token.balanceOf(hostAddress)
     let startBalanceReward = await token.balanceOf(rewardRecipient)
     let startBalanceCollateral = await token.balanceOf(collateralRecipient)
-    discard await marketplace.freeSlot(slotId, rewardRecipient, collateralRecipient)
+    discard await marketplace.freeSlot(slotId, rewardRecipient, collateralRecipient).confirm(1)
     let endBalanceHost = await token.balanceOf(hostAddress)
     let endBalanceReward = await token.balanceOf(rewardRecipient)
     let endBalanceCollateral = await token.balanceOf(collateralRecipient)
@@ -120,4 +120,5 @@ ethersuite "Marketplace contracts":
     await ethProvider.advanceTime(periodicity.seconds)
     check await marketplace
       .markProofAsMissing(slotId, missingPeriod)
+      .confirm(1)
       .reverts("Slot not accepting proofs")

tests/contracts/testMarket.nim

@@ -114,8 +114,7 @@ ethersuite "On-Chain Market":
       receivedAsks.add(ask)
     let subscription = await market.subscribeRequests(onRequest)
     await market.requestStorage(request)
-    check receivedIds == @[request.id]
-    check receivedAsks == @[request.ask]
+    check eventually receivedIds == @[request.id] and receivedAsks == @[request.ask]
     await subscription.unsubscribe()
 
   test "supports filling of slots":
@@ -181,8 +180,7 @@ ethersuite "On-Chain Market":
     let subscription = await market.subscribeSlotFilled(onSlotFilled)
     await market.reserveSlot(request.id, slotIndex)
     await market.fillSlot(request.id, slotIndex, proof, request.ask.collateral)
-    check receivedIds == @[request.id]
-    check receivedSlotIndices == @[slotIndex]
+    check eventually receivedIds == @[request.id] and receivedSlotIndices == @[slotIndex]
     await subscription.unsubscribe()
 
   test "subscribes only to a certain slot":
@@ -194,10 +192,9 @@ ethersuite "On-Chain Market":
     let subscription = await market.subscribeSlotFilled(request.id, slotIndex, onSlotFilled)
     await market.reserveSlot(request.id, otherSlot)
     await market.fillSlot(request.id, otherSlot, proof, request.ask.collateral)
-    check receivedSlotIndices.len == 0
     await market.reserveSlot(request.id, slotIndex)
     await market.fillSlot(request.id, slotIndex, proof, request.ask.collateral)
-    check receivedSlotIndices == @[slotIndex]
+    check eventually receivedSlotIndices == @[slotIndex]
     await subscription.unsubscribe()
 
   test "supports slot freed subscriptions":
@@ -211,8 +208,7 @@ ethersuite "On-Chain Market":
       receivedIdxs.add(idx)
     let subscription = await market.subscribeSlotFreed(onSlotFreed)
     await market.freeSlot(slotId(request.id, slotIndex))
-    check receivedRequestIds == @[request.id]
-    check receivedIdxs == @[slotIndex]
+    check eventually receivedRequestIds == @[request.id] and receivedIdxs == @[slotIndex]
     await subscription.unsubscribe()
 
   test "supports slot reservations full subscriptions":
@@ -235,8 +231,7 @@ ethersuite "On-Chain Market":
     switchAccount(account3)
     await market.reserveSlot(request.id, slotIndex)
 
-    check receivedRequestIds == @[request.id]
-    check receivedIdxs == @[slotIndex]
+    check eventually receivedRequestIds == @[request.id] and receivedIdxs == @[slotIndex]
     await subscription.unsubscribe()
 
   test "support fulfillment subscriptions":
@@ -248,7 +243,7 @@ ethersuite "On-Chain Market":
     for slotIndex in 0..<request.ask.slots:
       await market.reserveSlot(request.id, slotIndex.u256)
       await market.fillSlot(request.id, slotIndex.u256, proof, request.ask.collateral)
-    check receivedIds == @[request.id]
+    check eventually receivedIds == @[request.id]
     await subscription.unsubscribe()
 
   test "subscribes only to fulfillment of a certain request":
@@ -271,7 +266,7 @@ ethersuite "On-Chain Market":
       await market.reserveSlot(otherRequest.id, slotIndex.u256)
       await market.fillSlot(otherRequest.id, slotIndex.u256, proof, otherRequest.ask.collateral)
 
-    check receivedIds == @[request.id]
+    check eventually receivedIds == @[request.id]
 
     await subscription.unsubscribe()
 
@@ -285,7 +280,7 @@ ethersuite "On-Chain Market":
 
     await advanceToCancelledRequest(request)
     await market.withdrawFunds(request.id)
-    check receivedIds == @[request.id]
+    check eventually receivedIds == @[request.id]
     await subscription.unsubscribe()
 
   test "support request failed subscriptions":
@@ -308,8 +303,8 @@ ethersuite "On-Chain Market":
         await waitUntilProofRequired(slotId)
         let missingPeriod = periodicity.periodOf(await ethProvider.currentTime())
         await advanceToNextPeriod()
-        discard await marketplace.markProofAsMissing(slotId, missingPeriod)
-    check receivedIds == @[request.id]
+        discard await marketplace.markProofAsMissing(slotId, missingPeriod).confirm(1)
+    check eventually receivedIds == @[request.id]
     await subscription.unsubscribe()
 
   test "subscribes only to a certain request cancellation":
@@ -325,9 +320,8 @@ ethersuite "On-Chain Market":
     let subscription = await market.subscribeRequestCancelled(request.id, onRequestCancelled)
     await advanceToCancelledRequest(otherRequest) # shares expiry with otherRequest
     await market.withdrawFunds(otherRequest.id)
-    check receivedIds.len == 0
     await market.withdrawFunds(request.id)
-    check receivedIds == @[request.id]
+    check eventually receivedIds == @[request.id]
     await subscription.unsubscribe()
 
   test "supports proof submission subscriptions":
@@ -340,7 +334,7 @@ ethersuite "On-Chain Market":
       receivedIds.add(id)
     let subscription = await market.subscribeProofSubmission(onProofSubmission)
     await market.submitProof(slotId(request.id, slotIndex), proof)
-    check receivedIds == @[slotId(request.id, slotIndex)]
+    check eventually receivedIds == @[slotId(request.id, slotIndex)]
     await subscription.unsubscribe()
 
   test "request is none when unknown":

tests/contracts/time.nim

@@ -1,8 +1,11 @@
 import pkg/ethers
 import pkg/serde/json
 
+proc blockTime*(provider: Provider, tag: BlockTag): Future[UInt256] {.async.} =
+  return (!await provider.getBlock(tag)).timestamp
+
 proc currentTime*(provider: Provider): Future[UInt256] {.async.} =
-  return (!await provider.getBlock(BlockTag.pending)).timestamp
+  return await provider.blockTime(BlockTag.pending)
 
 proc advanceTime*(provider: JsonRpcProvider, seconds: UInt256) {.async.} =
   discard await provider.send("evm_increaseTime", @[%("0x" & seconds.toHex)])

tests/ethertest.nim

@@ -1,5 +1,6 @@
 import std/json
 import pkg/ethers
+import pkg/chronos
 
 import ./asynctest
 import ./checktest
@@ -16,11 +17,15 @@ template ethersuite*(name, body) =
     var snapshot: JsonNode
 
     setup:
-      ethProvider = JsonRpcProvider.new("ws://localhost:8545")
+      ethProvider = JsonRpcProvider.new(
+        "http://127.0.0.1:8545",
+        pollingInterval = chronos.milliseconds(100)
+      )
       snapshot = await send(ethProvider, "evm_snapshot")
       accounts = await ethProvider.listAccounts()
 
     teardown:
+      await ethProvider.close()
       discard await send(ethProvider, "evm_revert", @[snapshot])
 
     body

tests/integration/multinodes.nim

@@ -196,13 +196,15 @@ template multinodesuite*(name: string, body: untyped) =
     proc startClientNode(conf: CodexConfig): Future[NodeProcess] {.async.} =
       let clientIdx = clients().len
       var config = conf
+      config.addCliOption(StartUpCmd.persistence, "--eth-provider", "http://127.0.0.1:8545")
       config.addCliOption(StartUpCmd.persistence, "--eth-account", $accounts[running.len])
       return await newCodexProcess(clientIdx, config, Role.Client)
 
     proc startProviderNode(conf: CodexConfig): Future[NodeProcess] {.async.} =
       let providerIdx = providers().len
       var config = conf
       config.addCliOption("--bootstrap-node", bootstrap)
+      config.addCliOption(StartUpCmd.persistence, "--eth-provider", "http://127.0.0.1:8545")
       config.addCliOption(StartUpCmd.persistence, "--eth-account", $accounts[running.len])
       config.addCliOption(PersistenceCmd.prover, "--circom-r1cs",
         "vendor/codex-contracts-eth/verifier/networks/hardhat/proof_main.r1cs")
@@ -217,6 +219,7 @@ template multinodesuite*(name: string, body: untyped) =
       let validatorIdx = validators().len
       var config = conf
       config.addCliOption("--bootstrap-node", bootstrap)
+      config.addCliOption(StartUpCmd.persistence, "--eth-provider", "http://127.0.0.1:8545")
       config.addCliOption(StartUpCmd.persistence, "--eth-account", $accounts[running.len])
       config.addCliOption(StartUpCmd.persistence, "--validator")
 
@@ -264,7 +267,10 @@ template multinodesuite*(name: string, body: untyped) =
         # Workaround for https://github.com/NomicFoundation/hardhat/issues/2053
         # Do not use websockets, but use http and polling to stop subscriptions
         # from being removed after 5 minutes
-        ethProvider = JsonRpcProvider.new("http://localhost:8545")
+        ethProvider = JsonRpcProvider.new(
+          "http://127.0.0.1:8545",
+          pollingInterval = chronos.milliseconds(100)
+        )
         # if hardhat was NOT started by the test, take a snapshot so it can be
         # reverted in the test teardown
         if nodeConfigs.hardhat.isNone:

tests/integration/testecbug.nim

@@ -23,9 +23,9 @@ marketplacesuite "Bug #821 - node crashes during erasure coding":
           .some,
   ):
     let reward = 400.u256
-    let duration = 10.periods
+    let duration = 20.periods
     let collateral = 200.u256
-    let expiry = 5.periods
+    let expiry = 10.periods
     let data = await RandomChunker.example(blocks=8)
     let client = clients()[0]
     let clientApi = client.client