From 373ecba971abea9710e0ba790c1a9f7079c86e51 Mon Sep 17 00:00:00 2001 From: mwc Date: Sun, 8 Jan 2023 12:29:01 -0500 Subject: [PATCH 1/2] proposed changes in light that max index is special case for prefix suim --- tests/forge/FenwickTree.t.sol | 13 ++++++++++--- tests/forge/utils/FenwickTreeInstance.sol | 10 ++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/tests/forge/FenwickTree.t.sol b/tests/forge/FenwickTree.t.sol index 1ec832bd8..90d93f687 100644 --- a/tests/forge/FenwickTree.t.sol +++ b/tests/forge/FenwickTree.t.sol @@ -193,14 +193,19 @@ contract FenwickTreeTest is DSTestPlus { // uint256 factor_ Bound Result: 1000000000000000000 _tree.nonFuzzyFill(1145, 549824577419048462197751701941564, 1746721984912593317128871914298176971615500961962358, false); - uint256 scaleIndex = 7388; + uint256 scaleIndex = 7388; // MAX index of the Fenwick tree uint256 subIndex = randomInRange(0, 7388 - 1); uint256 factor = 1000000000000000000; _tree.mult(scaleIndex, factor); // This offset is done because of a rounding issue that occurs when we calculate the prefixSum + // Because scaleIndex == MAX_INDEX, this will end up being scaleIndex - 1 uint256 treeDirectedIndex = _tree.findIndexOfSum(_tree.prefixSum(scaleIndex) + 1) - 1; + assertEq(scaleIndex, treeDirectedIndex + 1); + + // assuming subIndex < MAX, this should be the largest index >= subIndex with no additional + // deposit between it and subindex uint256 treeDirectedSubIndex = _tree.findIndexOfSum(_tree.prefixSum(subIndex) + 1) - 1; uint256 max = Maths.max(_tree.prefixSum(treeDirectedIndex), _tree.prefixSum(scaleIndex)); @@ -209,8 +214,8 @@ contract FenwickTreeTest is DSTestPlus { uint256 subMax = Maths.max(_tree.prefixSum(treeDirectedSubIndex), _tree.prefixSum(subIndex)); uint256 subMin = Maths.min(_tree.prefixSum(treeDirectedSubIndex), _tree.prefixSum(subIndex)); + assertEq(max - min, _tree.valueAt(scaleIndex)); // 2 >= scaling discrepency - assertLe(max - min, 2); assertLe(subMax - subMin, 2); } @@ -230,6 +235,7 @@ contract FenwickTreeTest is DSTestPlus { // This offset is done because of a rounding issue that occurs when we calculate the prefixSum uint256 treeDirectedIndex = _tree.findIndexOfSum(_tree.prefixSum(scaleIndex) + 1) - 1; + assertEq(scaleIndex, treeDirectedIndex + 1); uint256 treeDirectedSubIndex = _tree.findIndexOfSum(_tree.prefixSum(subIndex) + 1) - 1; uint256 max = Maths.max(_tree.prefixSum(treeDirectedIndex), _tree.prefixSum(scaleIndex)); @@ -238,8 +244,9 @@ contract FenwickTreeTest is DSTestPlus { uint256 subMax = Maths.max(_tree.prefixSum(treeDirectedSubIndex), _tree.prefixSum(subIndex)); uint256 subMin = Maths.min(_tree.prefixSum(treeDirectedSubIndex), _tree.prefixSum(subIndex)); + assertEq(max - min, _tree.valueAt(scaleIndex)); // 2 >= scaling discrepency - assertLe(max - min, 2); + // assertLe(max - min, 2); assertLe(subMax - subMin, 2); } diff --git a/tests/forge/utils/FenwickTreeInstance.sol b/tests/forge/utils/FenwickTreeInstance.sol index 14ef06944..0c50da369 100644 --- a/tests/forge/utils/FenwickTreeInstance.sol +++ b/tests/forge/utils/FenwickTreeInstance.sol @@ -119,6 +119,7 @@ contract FenwickTreeInstance is DSTestPlus { ) external { uint256 i; uint256 amount; + uint256 cumulativeAmount; // Initialize and print seed for randomness setRandomSeed(seed_); @@ -133,15 +134,16 @@ contract FenwickTreeInstance is DSTestPlus { // Update values add(i, amount); - amount_ -= amount; - insertions_ -= 1; + amount_ -= amount; + insertions_ -= 1; + cumulativeAmount += amount; // Verify tree sum - assertEq(deposits.treeSum(), amount_ - amount_); + assertEq(deposits.treeSum(), cumulativeAmount); if (trackInserts) inserts.push(i); } - assertEq(deposits.treeSum(), amount_); + assertEq(deposits.treeSum(), cumulativeAmount); } } From 71a92712c2a76513c645c9d2ad457ffbfcdba91c Mon Sep 17 00:00:00 2001 From: Ed Noepel Date: Sun, 8 Jan 2023 16:06:22 -0500 Subject: [PATCH 2/2] removed comment --- tests/forge/FenwickTree.t.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/forge/FenwickTree.t.sol b/tests/forge/FenwickTree.t.sol index 90d93f687..76da495a8 100644 --- a/tests/forge/FenwickTree.t.sol +++ b/tests/forge/FenwickTree.t.sol @@ -246,7 +246,6 @@ contract FenwickTreeTest is DSTestPlus { assertEq(max - min, _tree.valueAt(scaleIndex)); // 2 >= scaling discrepency - // assertLe(max - min, 2); assertLe(subMax - subMin, 2); }