safeMulDiv round (#1187)

reserve-protocol · Aug 29, 2024 · 468df05 · 468df05
1 parent 5102082
commit 468df05
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 3 deletions.
diff --git a/contracts/libraries/Fixed.sol b/contracts/libraries/Fixed.sol
@@ -577,8 +577,6 @@ library FixLib {
             uint256 pow2 = c & (0 - c);
 
             uint256 c_256 = uint256(c);
-            // Warning: Should not access c below this line
-
             c_256 /= pow2;
             lo /= pow2;
             lo += hi * ((0 - pow2) / pow2 + 1);
@@ -597,7 +595,7 @@ library FixLib {
             if (rounding == CEIL) {
                 if (mm != 0) result_256 += 1;
             } else if (rounding == ROUND) {
-                if (mm > ((c_256 - 1) / 2)) result_256 += 1;
+                if (mm > ((c - 1) / 2)) result_256 += 1; // intentional: use pre-divided c here
             }
         }
 

diff --git a/test/libraries/Fixed.test.ts b/test/libraries/Fixed.test.ts
@@ -1145,6 +1145,16 @@ describe('In FixLib,', () => {
         await caller.safeMulDiv(MAX_UINT192.div(fp(2)).sub(1), fp(1), MAX_UINT192, CEIL)
       ).to.equal(1)
     })
+
+    it('regression test -- safeMulDiv with ROUND', async () => {
+      // ROUND was rounding up for _most_ values, instead of half of them
+      // here we test that 0.1, 0.01, ...0.000000000000001 etc are all rounded down
+
+      for (let i = 0; i < 18; i++) {
+        const amt = fp(`1e-${i + 1}`)
+        expect(await caller.safeMulDiv(amt, fp(1).add(1), fp(1), ROUND)).to.equal(amt)
+      }
+    })
   })
 
   describe('safeDiv_', () => {