fix optimization test and inline draw ID

GenerationSoftware · Oct 4, 2023 · 9e6255b · 9e6255b
1 parent 78e403a
commit 9e6255b
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 8 deletions.
diff --git a/src/libraries/DrawAccumulatorLib.sol b/src/libraries/DrawAccumulatorLib.sol
@@ -93,8 +93,7 @@ library DrawAccumulatorLib {
         cardinality += 1;
       } else {
         // Delete the old observation to save gas (older than 1 year)
-        uint24 overwritingDrawId = accumulator.drawRingBuffer[ringBufferInfo.nextIndex];
-        delete accumulatorObservations[overwritingDrawId];
+        delete accumulatorObservations[accumulator.drawRingBuffer[ringBufferInfo.nextIndex]];
       }
 
       accumulator.drawRingBuffer[ringBufferInfo.nextIndex] = _drawId;

diff --git a/test/libraries/DrawAccumulatorLib.t.sol b/test/libraries/DrawAccumulatorLib.t.sol
@@ -77,14 +77,31 @@ contract DrawAccumulatorLibTest is Test {
 
   function testAddOne_deleteExpired() public {
     // set up accumulator as if we had just completed a buffer loop:
-    accumulator.drawRingBuffer[0] = 101; // set to some draw ID that isn't 0 or the one we'll be writing
-    accumulator.observations[101] = Observation(2, 3);
+    uint normalGasUsed;
+    for (uint24 i = 0; i < 366; i++) {
+      uint gasLeftStart = gasleft();
+      DrawAccumulatorLib.add(accumulator, 100, i + 1, alpha);
+      if (i == 0) {
+        normalGasUsed = gasLeftStart - gasleft();
+      }
+      assertEq(accumulator.ringBufferInfo.cardinality, i + 1);
+      assertEq(accumulator.ringBufferInfo.nextIndex, i == 365 ? 0 : i + 1);
+      assertEq(accumulator.drawRingBuffer[i], i + 1);
+      assertGe(accumulator.observations[i + 1].available, accumulator.observations[i].available);
+    }
 
-    DrawAccumulatorLib.add(accumulator, 100, 1, alpha);
-    assertEq(accumulator.ringBufferInfo.cardinality, 1);
+    assertEq(accumulator.ringBufferInfo.cardinality, 366);
+
+    uint gasLeftStart2 = gasleft();
+    DrawAccumulatorLib.add(accumulator, 200, 367, alpha);
+    uint gasUsed = gasLeftStart2 - gasleft();
+    assertLt(gasUsed, normalGasUsed - 15000); // should now cost at least 15k less gas since we are deleting some data
+
+    assertEq(accumulator.ringBufferInfo.cardinality, 366);
     assertEq(accumulator.ringBufferInfo.nextIndex, 1);
-    assertEq(accumulator.drawRingBuffer[0], 1);
-    assertEq(accumulator.observations[1].available, 100);
+    assertEq(accumulator.drawRingBuffer[0], 367);
+    assertGt(accumulator.observations[367].available, accumulator.observations[366].available);
+    assertEq(accumulator.observations[1].available, 0); // deleted draw 1
   }
 
   function testGetTotalRemaining() public {