Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

proposed changes in light that max index is special case for prefix suim #533

Merged
merged 2 commits into from
Jan 8, 2023
Merged

proposed changes in light that max index is special case for prefix suim #533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
373ecba
proposed changes in light that max index is special case for prefix suim
Jan 8, 2023
71a9271
removed comment
Jan 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions tests/forge/FenwickTree.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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);
}

Expand All @@ -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));
Expand All @@ -238,8 +244,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);
}

Expand Down
10 changes: 6 additions & 4 deletions tests/forge/utils/FenwickTreeInstance.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ contract FenwickTreeInstance is DSTestPlus {
) external {
uint256 i;
uint256 amount;
uint256 cumulativeAmount;

// Initialize and print seed for randomness
setRandomSeed(seed_);
Expand All @@ -133,15 +134,16 @@ contract FenwickTreeInstance is DSTestPlus {

// Update values
add(i, amount);
amount_ -= amount;
insertions_ -= 1;
amount_ -= amount;
EdNoepel marked this conversation as resolved.
Show resolved Hide resolved
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);
}
}