adds mockprovider to simplify and improve testing of the edge conditions

codex/contracts/market.nim

@@ -1,4 +1,5 @@
 import std/strutils
+import std/times
 import pkg/ethers
 import pkg/upraises
 import pkg/questionable
@@ -410,17 +411,6 @@ proc blockNumberAndTimestamp*(provider: Provider, blockTag: BlockTag):
 
   (latestBlockNumber, latestBlock.timestamp)
 
-proc estimateAverageBlockTime*(provider: Provider): Future[UInt256] {.async.} =
-  let (latestBlockNumber, latestBlockTimestamp) =
-    await provider.blockNumberAndTimestamp(BlockTag.latest)
-  let (_, previousBlockTimestamp) =
-    await provider.blockNumberAndTimestamp(
-      BlockTag.init(latestBlockNumber - 1.u256))
-  debug "[estimateAverageBlockTime]:", latestBlockNumber = latestBlockNumber,
-    latestBlockTimestamp = latestBlockTimestamp,
-    previousBlockTimestamp = previousBlockTimestamp
-  return latestBlockTimestamp - previousBlockTimestamp
-
 proc binarySearchFindClosestBlock*(provider: Provider,
                                   epochTime: int,
                                   low: UInt256,
@@ -468,8 +458,6 @@ proc binarySearchBlockNumberForEpoch*(provider: Provider,
 
 proc blockNumberForEpoch*(provider: Provider,
     epochTime: SecondsSince1970): Future[UInt256] {.async.} =
-  let avgBlockTime = await provider.estimateAverageBlockTime()
-  debug "[blockNumberForEpoch]:", avgBlockTime = avgBlockTime
   debug "[blockNumberForEpoch]:", epochTime = epochTime
   let epochTimeUInt256 = epochTime.u256
   let (latestBlockNumber, latestBlockTimestamp) = 
@@ -482,12 +470,45 @@ proc blockNumberForEpoch*(provider: Provider,
   debug "[blockNumberForEpoch]:", earliestBlockNumber = earliestBlockNumber,
     earliestBlockTimestamp = earliestBlockTimestamp
 
-  let timeDiff = latestBlockTimestamp - epochTimeUInt256
-  let blockDiff = timeDiff div avgBlockTime
-
-  debug "[blockNumberForEpoch]:", timeDiff = timeDiff, blockDiff = blockDiff
-
-  if blockDiff >= latestBlockNumber - earliestBlockNumber:
+  # Initially we used the average block time to predict
+  # the number of blocks we need to look back in order to find
+  # the block number corresponding to the given epoch time. 
+  # This estimation can be highly inaccurate if block time
+  # was changing in the past or is fluctuating and therefore
+  # we used that information initially only to find out
+  # if the available history is long enough to perform effective search.
+  # It turns out we do not have to do that. There is an easier way.
+  #
+  # First we check if the given epoch time equals the timestamp of either
+  # the earliest or the latest block. If it does, we just return the
+  # block number of that block.
+  #
+  # Otherwise, if the earliest available block is not the genesis block, 
+  # we should check the timestamp of that earliest block and if it is greater
+  # than the epoch time, we should issue a warning and return
+  # that earliest block number.
+  # In all other cases, thus when the earliest block is not the genesis
+  # block but its timestamp is not greater than the requested epoch time, or
+  # if the earliest available block is the genesis block, 
+  # (which means we have the whole history available), we should proceed with
+  # the binary search.
+  #
+  # Additional benefit of this method is that we do not have to rely
+  # on the average block time, which not only makes the whole thing
+  # more reliable, but also easier to test.
+
+  # Are lucky today?
+  if earliestBlockTimestamp == epochTimeUInt256:
+    return earliestBlockNumber
+  if latestBlockTimestamp == epochTimeUInt256:
+    return latestBlockNumber
+
+  if earliestBlockNumber > 0 and earliestBlockTimestamp > epochTimeUInt256:
+    let availableHistoryInDays = 
+        (latestBlockTimestamp - earliestBlockTimestamp) div
+          initDuration(days = 1).inSeconds.u256
+    warn "Short block history detected.", earliestBlockTimestamp =  
+      earliestBlockTimestamp, days = availableHistoryInDays
     return earliestBlockNumber
 
   return await provider.binarySearchBlockNumberForEpoch(

tests/contracts/helpers/mockprovider.nim

@@ -0,0 +1,79 @@
+import std/strutils
+import std/tables
+
+import pkg/ethers/provider
+from codex/clock import SecondsSince1970
+
+export provider.Block
+
+type MockProvider* = ref object of Provider
+  blocks: OrderedTableRef[int, Block]
+  earliest: ?int
+  latest: ?int
+
+method getBlock*(
+  provider: MockProvider,
+  tag: BlockTag
+): Future[?Block] {.async.} =
+  if $tag == "latest":
+    if latestBlock =? provider.latest:
+      if provider.blocks.hasKey(latestBlock):
+        return provider.blocks[latestBlock].some
+  elif $tag == "earliest":
+    if earliestBlock =? provider.earliest:
+      if provider.blocks.hasKey(earliestBlock):
+        return provider.blocks[earliestBlock].some
+  else:
+    let blockNumber = parseHexInt($tag)
+    if provider.blocks.hasKey(blockNumber):
+      return provider.blocks[blockNumber].some
+  return Block.none
+
+proc updateEarliestAndLatest(provider: MockProvider, blockNumber: int) =
+  if provider.earliest.isNone:
+    provider.earliest = blockNumber.some
+  provider.latest = blockNumber.some
+
+proc addBlocks*(provider: MockProvider, blocks: OrderedTableRef[int, Block]) =
+  for number, blk in blocks.pairs:
+    if provider.blocks.hasKey(number):
+      continue
+    provider.updateEarliestAndLatest(number)
+    provider.blocks[number] = blk
+
+proc addBlock*(provider: MockProvider, number: int, blk: Block) =
+  if not provider.blocks.hasKey(number):
+    provider.updateEarliestAndLatest(number)
+    provider.blocks[number] = blk
+
+proc newMockProvider*(): MockProvider =
+  MockProvider(
+    blocks: newOrderedTable[int, Block](),
+    earliest: int.none,
+    latest: int.none
+  )
+
+proc newMockProvider*(blocks: OrderedTableRef[int, Block]): MockProvider =
+  let provider = newMockProvider()
+  provider.addBlocks(blocks)
+  provider
+
+proc newMockProvider*(
+  numberOfBlocks: int, 
+  earliestBlockNumber: int,
+  earliestBlockTimestamp: SecondsSince1970,
+  timeIntervalBetweenBlocks: SecondsSince1970
+): MockProvider =
+  var blocks = newOrderedTable[int, provider.Block]()
+  var blockNumber = earliestBlockNumber
+  var blockTime = earliestBlockTimestamp
+  for i in 0..<numberOfBlocks:
+    blocks[blockNumber] = provider.Block(number: blockNumber.u256.some,
+      timestamp: blockTime.u256, hash: BlockHash.none)
+    inc blockNumber
+    inc blockTime, timeIntervalBetweenBlocks.int
+  MockProvider(
+    blocks: blocks,
+    earliest: earliestBlockNumber.some,
+    latest: (earliestBlockNumber + numberOfBlocks - 1).some
+  )

tests/contracts/testMarket.nim

@@ -7,6 +7,7 @@ import ../ethertest
 import ./examples
 import ./time
 import ./deployment
+import ./helpers/mockProvider
 
 privateAccess(OnChainMarket) # enable access to private fields
 
@@ -494,42 +495,65 @@ ethersuite "On-Chain Market":
 
     check events.len == 0
 
-  test "estimateAverageBlockTime correctly computes the time between " &
-       "two most recent blocks":
-    let simulatedBlockTime = 15.u256
-    await ethProvider.mineNBlocks(1)
-    let (_, timestampPrevious) =
-      await ethProvider.blockNumberAndTimestamp(BlockTag.latest)
-
-    await ethProvider.advanceTime(simulatedBlockTime)
+  test "blockNumberForEpoch returns the earliest block when its timestamp " &
+       "is greater than the given epoch time and the earliest block is not " &
+       "block number 0 (genesis block)":
+    let mockProvider = newMockProvider(
+      numberOfBlocks = 10,
+      earliestBlockNumber = 1,
+      earliestBlockTimestamp = 10,
+      timeIntervalBetweenBlocks = 10
+    )
 
-    let (_, timestampLatest) =
-      await ethProvider.blockNumberAndTimestamp(BlockTag.latest)
+    let (earliestBlockNumber, earliestTimestamp) =
+      await mockProvider.blockNumberAndTimestamp(BlockTag.earliest)
 
-    let expected = timestampLatest - timestampPrevious
-    let actual = await ethProvider.estimateAverageBlockTime()
+    let epochTime = earliestTimestamp - 1
+
+    let actual = await mockProvider.blockNumberForEpoch(
+      epochTime.truncate(SecondsSince1970))
 
-    check expected == simulatedBlockTime
-    check actual == expected
+    check actual == earliestBlockNumber
 
-  test "blockNumberForEpoch returns the earliest block when retained history " &
-       "is shorter than the given epoch time":
-    # create predictable conditions
-    # we keep minimal resultion of 1s so that we are sure that
-    # we will land before the earliest (genesis in our case) block
-    let averageBlockTime = 1.u256
-    await ethProvider.mineNBlocks(1)
-    await ethProvider.advanceTime(averageBlockTime)
+  test "blockNumberForEpoch returns the earliest block when its timestamp " &
+       "is equal to the given epoch time":
+    let mockProvider = newMockProvider(
+      numberOfBlocks = 10,
+      earliestBlockNumber = 0,
+      earliestBlockTimestamp = 10,
+      timeIntervalBetweenBlocks = 10
+    )
+
     let (earliestBlockNumber, earliestTimestamp) =
-      await ethProvider.blockNumberAndTimestamp(BlockTag.earliest)
+      await mockProvider.blockNumberAndTimestamp(BlockTag.earliest)
 
-    let fromTime = earliestTimestamp - 1
+    let epochTime = earliestTimestamp
 
-    let actual = await ethProvider.blockNumberForEpoch(
-      fromTime.truncate(SecondsSince1970))
+    let actual = await mockProvider.blockNumberForEpoch(
+      epochTime.truncate(SecondsSince1970))
 
+    check earliestBlockNumber == 0.u256
     check actual == earliestBlockNumber
 
+  test "blockNumberForEpoch returns the latest block when its timestamp " &
+       "is equal to the given epoch time":
+    let mockProvider = newMockProvider(
+      numberOfBlocks = 10,
+      earliestBlockNumber = 0,
+      earliestBlockTimestamp = 10,
+      timeIntervalBetweenBlocks = 10
+    )
+
+    let (latestBlockNumber, latestTimestamp) =
+      await mockProvider.blockNumberAndTimestamp(BlockTag.latest)
+
+    let epochTime = latestTimestamp
+
+    let actual = await mockProvider.blockNumberForEpoch(
+      epochTime.truncate(SecondsSince1970))
+
+    check actual == latestBlockNumber
+
   test "blockNumberForEpoch finds closest blockNumber for given epoch time":
     proc createBlockHistory(n: int, blockTime: int):
         Future[seq[(UInt256, UInt256)]] {.async.} =