Fix defect in uTP buffer (#493)

eth/utp/growable_buffer.nim

@@ -19,32 +19,32 @@ export options
 type
   GrowableCircularBuffer*[A] = object
     items: seq[Option[A]]
-    mask: int
+    mask: uint32
 
 # provided size will always be adjusted to next power of two
-proc init*[A](T: type GrowableCircularBuffer[A], size: Natural = 16): T =
+proc init*[A](T: type GrowableCircularBuffer[A], size: uint32 = 16): T =
-  let powOfTwoSize = nextPowerOfTwo(size)
+  let powOfTwoSize = nextPowerOfTwo(int(size))
   T(
-    items: newSeq[Option[A]](size),
+    items: newSeq[Option[A]](powOfTwoSize),
-    mask: powOfTwoSize - 1
+    mask: uint32(powOfTwoSize - 1)
   )
 
-proc get*[A](buff: GrowableCircularBuffer[A], i: Natural): Option[A] =
+proc get*[A](buff: GrowableCircularBuffer[A], i: uint32): Option[A] =
   buff.items[i and buff.mask]
 
-proc putImpl[A](buff: var GrowableCircularBuffer[A], i: Natural, elem: Option[A]) =
+proc putImpl[A](buff: var GrowableCircularBuffer[A], i: uint32, elem: Option[A]) =
   buff.items[i and buff.mask] = elem
 
-proc put*[A](buff: var GrowableCircularBuffer[A], i: Natural, elem: A) =
+proc put*[A](buff: var GrowableCircularBuffer[A], i: uint32, elem: A) =
   buff.putImpl(i, some(elem))
 
-proc delete*[A](buff: var GrowableCircularBuffer[A], i: Natural) =
+proc delete*[A](buff: var GrowableCircularBuffer[A], i: uint32) =
   buff.putImpl(i, none[A]())
 
-proc hasKey*[A](buff: GrowableCircularBuffer[A], i: Natural): bool =
+proc hasKey*[A](buff: GrowableCircularBuffer[A], i: uint32): bool =
   buff.get(i).isSome()
 
-proc exists*[A](buff: GrowableCircularBuffer[A], i: Natural, check: proc (x: A): bool): bool =
+proc exists*[A](buff: GrowableCircularBuffer[A], i: uint32, check: proc (x: A): bool): bool =
   let maybeElem = buff.get(i)
   if (maybeElem.isSome()):
     let elem = maybeElem.unsafeGet()
@@ -52,32 +52,33 @@ proc exists*[A](buff: GrowableCircularBuffer[A], i: Natural, check: proc (x: A):
   else:
     false
 
-proc `[]`*[A](buff: var GrowableCircularBuffer[A], i: Natural): var A =
+proc `[]`*[A](buff: var GrowableCircularBuffer[A], i: uint32): var A =
   ## Returns contents of the `var GrowableCircularBuffer`. If it is not set, then an exception
   ## is thrown.
   buff.items[i and buff.mask].get()
 
 proc len*[A](buff: GrowableCircularBuffer[A]): int =
-  buff.mask + 1
+  int(buff.mask) + 1
+
+proc calculateNextMask(currentMask: uint32, index:uint32): uint32 =
+  # Increase mask,so that index will fit in buffer size i.e mask + 1
+  if currentMask == uint32.high:
+    return currentMask
+  
+  var newSize = currentMask + 1 
+  while true:
+    newSize = newSize * 2
+    if newSize == 0 or index < newSize:
+      break
+  return newSize - 1
 
 # Item contains the element we want to make space for
 # index is the index in the list.
-proc ensureSize*[A](buff: var GrowableCircularBuffer[A], item: Natural, index: Natural) =
+proc ensureSize*[A](buff: var GrowableCircularBuffer[A], item: uint32, index: uint32) =
-  # Increase size until is next power of 2 which consists given index
-  proc getNextSize(currentSize: int, index: int): int =
-    var newSize = currentSize
-    while true:
-      newSize = newSize * 2
-      if not (index >= newSize):
-        break
-    newSize
-
   if (index > buff.mask):
-    let currentSize = buff.mask + 1
+    let newMask = calculateNextMask(buff.mask, index)
-    let newSize = getNextSize(currentSize, index)
+    var newSeq = newSeq[Option[A]](int(newMask) + 1)
-    let newMask = newSize - 1
+    var i = 0'u32
-    var newSeq = newSeq[Option[A]](newSize)
-    var i = 0
     while i <= buff.mask:
       let idx = item - index + i
       newSeq[idx and newMask] = buff.get(idx)

tests/utp/test_buffer.nim

@@ -22,6 +22,18 @@ suite "Utp ring buffer":
       buff.len() == 4
       buff.get(0).isNone()
 
+  test "Buffer should be initialised to next power of two":
+    var elemsCounter = 0
+    let buff = GrowableCircularBuffer[int].init(size = 17) 
+    check:
+      buff.len() == 32
+
+    for i in buff.items:
+      inc elemsCounter  
+    
+    check:
+      elemsCounter == 32
+
   test "Adding elements to buffer":
     var buff = GrowableCircularBuffer[int].init(size = 4)
     buff.put(11, 11)
@@ -53,7 +65,7 @@ suite "Utp ring buffer":
   test "Checking if element exists and has some properties":
     var buff = GrowableCircularBuffer[TestObj].init(size = 4)
     let text = "test"
-    let textIdx = 11
+    let textIdx = 11'u32
 
     check:
       not buff.exists(textIdx, x => x.foo == text)
@@ -124,3 +136,39 @@ suite "Utp ring buffer":
       buff.get(15) == none[int]()
       buff.get(16) == none[int]()
       buff.get(17) == some(17)
+
+  test "Ensuring proper wrap around when adding elements to buffer":
+    var buff = GrowableCircularBuffer[int].init(size = 2)
+
+    buff.ensureSize(65535, 0)
+    buff.put(65535, 65535)
+    buff.ensureSize(0, 1)
+    buff.put(0, 0)
+
+    # index 2 will not fit in buffer of size 2 so it will need to be expanded to 4
+    buff.ensureSize(1, 2)
+    buff.put(1, 1)
+
+    check:
+      buff.len() == 4
+
+    buff.ensureSize(2, 3)
+    buff.put(2, 2)
+    # index 4 will not fit in buffer size 4 so it will need to expanded
+    buff.ensureSize(3, 4)
+    buff.put(3, 3)
+
+    # all elements should be available thorugh old indexes
+    check:
+      buff.get(65535) == some(65535)
+      buff.get(0) == some(0)
+      buff.get(1) == some(1)
+      buff.get(2) == some(2)
+      buff.get(3) == some(3)
+      buff.len() == 8
+    
+    var elemsCounter = 0
+    for elem in buff.items:
+      inc elemsCounter
+    check:
+      elemsCounter == 8