Merge pull request #27 from morpho-labs/refactor/naming-and-getters

Refactor/naming and getters

Author: MathisGD

src/LogarithmicBuckets.sol

@@ -6,8 +6,8 @@ import "./BucketDLL.sol";
 library LogarithmicBuckets {
     using BucketDLL for BucketDLL.List;
 
-    struct BucketList {
-        mapping(uint256 => BucketDLL.List) lists;
+    struct Buckets {
+        mapping(uint256 => BucketDLL.List) buckets;
         mapping(address => uint256) valueOf;
         uint256 bucketsMask;
     }
@@ -28,7 +28,7 @@ library LogarithmicBuckets {
     /// @param _newValue The new value of the account.
     /// @param _head Indicates whether to insert the new values at the head or at the tail of the buckets list.
     function update(
-        BucketList storage _buckets,
+        Buckets storage _buckets,
         address _id,
         uint256 _newValue,
         bool _head
@@ -56,41 +56,22 @@ library LogarithmicBuckets {
         }
     }
 
-    /// @notice Returns the value of `_id`.
-    /// @param _buckets The buckets to search in.
-    /// @param _id The address of the account.
-    function getValueOf(BucketList storage _buckets, address _id) internal view returns (uint256) {
-        return _buckets.valueOf[_id];
-    }
-
-    /// @notice Returns the bucket in which to look for `_value`.
-    /// @param _buckets The buckets to search in.
-    /// @param _value The value to look for.
-    function getBucketOf(BucketList storage _buckets, uint256 _value)
-        internal
-        view
-        returns (BucketDLL.List storage)
-    {
-        uint256 bucket = computeBucket(_value);
-        return _buckets.lists[bucket];
-    }
-
     /// @notice Returns the address in `_buckets` that is a candidate for matching the value `_value`.
     /// @param _buckets The buckets to get the head.
     /// @param _value The value to match.
     /// @return The address of the head.
-    function getMatch(BucketList storage _buckets, uint256 _value) internal view returns (address) {
+    function getMatch(Buckets storage _buckets, uint256 _value) internal view returns (address) {
         uint256 bucketsMask = _buckets.bucketsMask;
         if (bucketsMask == 0) return address(0);
         uint256 lowerMask = setLowerBits(_value);
 
         uint256 next = nextBucket(lowerMask, bucketsMask);
 
-        if (next != 0) return _buckets.lists[next].getHead();
+        if (next != 0) return _buckets.buckets[next].getHead();
 
         uint256 prev = prevBucket(lowerMask, bucketsMask);
 
-        return _buckets.lists[prev].getHead();
+        return _buckets.buckets[prev].getHead();
     }
 
     /// PRIVATE ///
@@ -101,11 +82,11 @@ library LogarithmicBuckets {
     /// @param _id The address of the account to remove.
     /// @param _bucket The mask of the bucket where to remove.
     function _remove(
-        BucketList storage _buckets,
+        Buckets storage _buckets,
         address _id,
         uint256 _bucket
     ) private {
-        if (_buckets.lists[_bucket].remove(_id))
+        if (_buckets.buckets[_bucket].remove(_id))
             _buckets.bucketsMask &= _bucket ^ type(uint256).max;
     }
 
@@ -117,14 +98,16 @@ library LogarithmicBuckets {
     /// @param _bucket The mask of the bucket where to insert.
     /// @param _head Whether to insert at the head or at the tail of the list.
     function _insert(
-        BucketList storage _buckets,
+        Buckets storage _buckets,
         address _id,
         uint256 _bucket,
         bool _head
     ) private {
-        if (_buckets.lists[_bucket].insert(_id, _head)) _buckets.bucketsMask |= _bucket;
+        if (_buckets.buckets[_bucket].insert(_id, _head)) _buckets.bucketsMask |= _bucket;
     }
 
+    /// PURE HELPERS ///
+
     /// @notice Returns the bucket in which the given value would fall.
     function computeBucket(uint256 _value) internal pure returns (uint256) {
         uint256 lowerMask = setLowerBits(_value);

test/TestLogarithmicBuckets.t.sol

@@ -6,7 +6,7 @@ import "./mocks/LogarithmicBucketsMock.sol";
 
 contract TestLogarithmicBuckets is LogarithmicBucketsMock, Test {
     using BucketDLL for BucketDLL.List;
-    using LogarithmicBuckets for LogarithmicBuckets.BucketList;
+    using LogarithmicBuckets for LogarithmicBuckets.Buckets;
 
     uint256 public accountsLength = 50;
     address[] public accounts;
@@ -21,30 +21,30 @@ contract TestLogarithmicBuckets is LogarithmicBucketsMock, Test {
     }
 
     function testInsertOneSingleAccount(bool _head) public {
-        bucketList.update(accounts[0], 3, _head);
+        buckets.update(accounts[0], 3, _head);
 
-        assertEq(bucketList.getValueOf(accounts[0]), 3);
-        assertEq(bucketList.getMatch(0), accounts[0]);
-        assertEq(bucketList.getBucketOf(3).getHead(), accounts[0]);
-        assertEq(bucketList.getBucketOf(2).getHead(), accounts[0]);
+        assertEq(buckets.valueOf[accounts[0]], 3);
+        assertEq(buckets.getMatch(0), accounts[0]);
+        assertEq(buckets.buckets[2].getHead(), accounts[0]);
+        assertEq(buckets.buckets[2].getHead(), accounts[0]);
     }
 
     function testUpdatingFromZeroToZeroShouldRevert(bool _head) public {
         vm.expectRevert(abi.encodeWithSignature("ZeroValue()"));
-        bucketList.update(accounts[0], 0, _head);
+        buckets.update(accounts[0], 0, _head);
     }
 
     function testShouldNotInsertZeroAddress(bool _head) public {
         vm.expectRevert(abi.encodeWithSignature("ZeroAddress()"));
-        bucketList.update(address(0), 10, _head);
+        buckets.update(address(0), 10, _head);
     }
 
     function testShouldHaveTheRightOrderWithinABucketFIFO() public {
-        bucketList.update(accounts[0], 16, false);
-        bucketList.update(accounts[1], 16, false);
-        bucketList.update(accounts[2], 16, false);
+        buckets.update(accounts[0], 16, false);
+        buckets.update(accounts[1], 16, false);
+        buckets.update(accounts[2], 16, false);
 
-        BucketDLL.List storage list = bucketList.getBucketOf(16);
+        BucketDLL.List storage list = buckets.buckets[16];
         address head = list.getNext(address(0));
         address next1 = list.getNext(head);
         address next2 = list.getNext(next1);
@@ -54,11 +54,11 @@ contract TestLogarithmicBuckets is LogarithmicBucketsMock, Test {
     }
 
     function testShouldHaveTheRightOrderWithinABucketLIFO() public {
-        bucketList.update(accounts[0], 16, true);
-        bucketList.update(accounts[1], 16, true);
-        bucketList.update(accounts[2], 16, true);
+        buckets.update(accounts[0], 16, true);
+        buckets.update(accounts[1], 16, true);
+        buckets.update(accounts[2], 16, true);
 
-        BucketDLL.List storage list = bucketList.getBucketOf(16);
+        BucketDLL.List storage list = buckets.buckets[16];
         address head = list.getNext(address(0));
         address next1 = list.getNext(head);
         address next2 = list.getNext(next1);
@@ -68,52 +68,52 @@ contract TestLogarithmicBuckets is LogarithmicBucketsMock, Test {
     }
 
     function testInsertRemoveOneSingleAccount(bool _head1, bool _head2) public {
-        bucketList.update(accounts[0], 1, _head1);
-        bucketList.update(accounts[0], 0, _head2);
+        buckets.update(accounts[0], 1, _head1);
+        buckets.update(accounts[0], 0, _head2);
 
-        assertEq(bucketList.getValueOf(accounts[0]), 0);
-        assertEq(bucketList.getMatch(0), address(0));
-        assertEq(bucketList.getBucketOf(1).getHead(), address(0));
+        assertEq(buckets.valueOf[accounts[0]], 0);
+        assertEq(buckets.getMatch(0), address(0));
+        assertEq(buckets.buckets[1].getHead(), address(0));
     }
 
     function testShouldInsertTwoAccounts(bool _head1, bool _head2) public {
-        bucketList.update(accounts[0], 16, _head1);
-        bucketList.update(accounts[1], 4, _head2);
+        buckets.update(accounts[0], 16, _head1);
+        buckets.update(accounts[1], 4, _head2);
 
-        assertEq(bucketList.getMatch(16), accounts[0]);
-        assertEq(bucketList.getMatch(2), accounts[1]);
-        assertEq(bucketList.getBucketOf(4).getHead(), accounts[1]);
+        assertEq(buckets.getMatch(16), accounts[0]);
+        assertEq(buckets.getMatch(2), accounts[1]);
+        assertEq(buckets.buckets[4].getHead(), accounts[1]);
     }
 
     function testShouldRemoveOneAccountOverTwo() public {
-        bucketList.update(accounts[0], 4, false);
-        bucketList.update(accounts[1], 16, false);
-        bucketList.update(accounts[0], 0, false);
-
-        assertEq(bucketList.getMatch(4), accounts[1]);
-        assertEq(bucketList.getValueOf(accounts[0]), 0);
-        assertEq(bucketList.getValueOf(accounts[1]), 16);
-        assertEq(bucketList.getBucketOf(16).getHead(), accounts[1]);
-        assertEq(bucketList.getBucketOf(4).getHead(), address(0));
+        buckets.update(accounts[0], 4, false);
+        buckets.update(accounts[1], 16, false);
+        buckets.update(accounts[0], 0, false);
+
+        assertEq(buckets.getMatch(4), accounts[1]);
+        assertEq(buckets.valueOf[accounts[0]], 0);
+        assertEq(buckets.valueOf[accounts[1]], 16);
+        assertEq(buckets.buckets[16].getHead(), accounts[1]);
+        assertEq(buckets.buckets[4].getHead(), address(0));
     }
 
     function testShouldRemoveBothAccounts() public {
-        bucketList.update(accounts[0], 4, true);
-        bucketList.update(accounts[1], 4, true);
-        bucketList.update(accounts[0], 0, true);
-        bucketList.update(accounts[1], 0, true);
+        buckets.update(accounts[0], 4, true);
+        buckets.update(accounts[1], 4, true);
+        buckets.update(accounts[0], 0, true);
+        buckets.update(accounts[1], 0, true);
 
-        assertEq(bucketList.getMatch(4), address(0));
+        assertEq(buckets.getMatch(4), address(0));
     }
 
     function testGetMatch() public {
-        assertEq(bucketList.getMatch(0), address(0));
-        assertEq(bucketList.getMatch(1000), address(0));
+        assertEq(buckets.getMatch(0), address(0));
+        assertEq(buckets.getMatch(1000), address(0));
 
-        bucketList.update(accounts[0], 16, false);
-        assertEq(bucketList.getMatch(1), accounts[0], "head before");
-        assertEq(bucketList.getMatch(16), accounts[0], "head equal");
-        assertEq(bucketList.getMatch(32), accounts[0], "head above");
+        buckets.update(accounts[0], 16, false);
+        assertEq(buckets.getMatch(1), accounts[0], "head before");
+        assertEq(buckets.getMatch(16), accounts[0], "head equal");
+        assertEq(buckets.getMatch(32), accounts[0], "head above");
     }
 
     function isPowerOfTwo(uint256 x) public pure returns (bool) {
@@ -134,8 +134,8 @@ contract TestLogarithmicBuckets is LogarithmicBucketsMock, Test {
 
     function testProveNextBucket(uint256 _value) public {
         uint256 curr = LogarithmicBuckets.computeBucket(_value);
-        uint256 next = nextBucket(_value);
-        uint256 bucketsMask = bucketList.bucketsMask;
+        uint256 next = nextBucketValue(_value);
+        uint256 bucketsMask = buckets.bucketsMask;
         // check that `next` is a strictly higer non-empty bucket, or zero
         assertTrue(next == 0 || isPowerOfTwo(next));
         assertTrue(next == 0 || next > curr);
@@ -151,8 +151,8 @@ contract TestLogarithmicBuckets is LogarithmicBucketsMock, Test {
 
     function testProvePrevBucket(uint256 _value) public {
         uint256 curr = LogarithmicBuckets.computeBucket(_value);
-        uint256 prev = prevBucket(_value);
-        uint256 bucketsMask = bucketList.bucketsMask;
+        uint256 prev = prevBucketValue(_value);
+        uint256 bucketsMask = buckets.bucketsMask;
         // check that `prev` is a non-empty bucket that is lower than or equal to `curr`; or zero
         assertTrue(prev == 0 || isPowerOfTwo(prev));
         assertTrue(prev <= curr);

test/TestLogarithmicBucketsGas.t.sol

@@ -5,9 +5,9 @@ import "forge-std/Test.sol";
 import "src/LogarithmicBuckets.sol";
 
 contract TestLogarithmicBucketsGas is Test {
-    using LogarithmicBuckets for LogarithmicBuckets.BucketList;
+    using LogarithmicBuckets for LogarithmicBuckets.Buckets;
 
-    LogarithmicBuckets.BucketList internal bucketList;
+    LogarithmicBuckets.Buckets internal buckets;
 
     // Gas accounting.
     uint256 internal insertCost;
@@ -33,19 +33,19 @@ contract TestLogarithmicBucketsGas is Test {
             if (amount % 4 < 2) {
                 // Measure insert.
                 startGasMetering();
-                bucketList.update(address(uint160(amount)), amount, true);
+                buckets.update(address(uint160(amount)), amount, true);
                 insertCost += stopGasMetering();
                 insertCount++;
             }
             // Update value in same bucket (p=1/4).
             else if (amount % 4 == 2) {
                 // Get an account to update its value.
-                address toUpdate = bucketList.getMatch(amount);
+                address toUpdate = buckets.getMatch(amount);
 
                 if (toUpdate != address(0)) {
                     // Measure updateValue.
                     startGasMetering();
-                    bucketList.update(toUpdate, amount, true);
+                    buckets.update(toUpdate, amount, true);
                     updateValueCost += stopGasMetering();
                     updateValueCount++;
                 }
@@ -54,14 +54,14 @@ contract TestLogarithmicBucketsGas is Test {
             else {
                 // Measure getMatch.
                 startGasMetering();
-                address toUpdate = bucketList.getMatch(amount);
+                address toUpdate = buckets.getMatch(amount);
                 getMatchCost += stopGasMetering();
                 getMatchCount++;
 
                 if (toUpdate != address(0)) {
                     // Measure remove.
                     startGasMetering();
-                    bucketList.update(bucketList.getMatch(amount), 0, true);
+                    buckets.update(buckets.getMatch(amount), 0, true);
                     removeCost += stopGasMetering();
                     removeCount++;
                 }

test/TestLogarithmicBucketsInvariant.t.sol

@@ -32,7 +32,7 @@ contract TestLogarithmicBucketsInvariant is Test, Random {
 
 contract LogarithmicBucketsSeenMock is LogarithmicBucketsMock {
     using BucketDLL for BucketDLL.List;
-    using LogarithmicBuckets for LogarithmicBuckets.BucketList;
+    using LogarithmicBuckets for LogarithmicBuckets.Buckets;
 
     address[] public seen;
     mapping(address => bool) public isSeen;
@@ -54,14 +54,12 @@ contract LogarithmicBucketsSeenMock is LogarithmicBucketsMock {
         super.update(_id, _newValue, _head);
     }
 
-    function getPrev(uint256 _value, address _id) public view returns (address) {
-        BucketDLL.List storage bucket = bucketList.getBucketOf(_value);
-        return bucket.getPrev(_id);
+    function getPrev(uint256 _bucket, address _id) public view returns (address) {
+        return buckets.buckets[_bucket].getPrev(_id);
     }
 
-    function getNext(uint256 _value, address _id) public view returns (address) {
-        BucketDLL.List storage bucket = bucketList.getBucketOf(_value);
-        return bucket.getNext(_id);
+    function getNext(uint256 _bucket, address _id) public view returns (address) {
+        return buckets.buckets[_bucket].getNext(_id);
     }
 }
 
@@ -77,10 +75,11 @@ contract TestLogarithmicBucketsSeenInvariant is Test, Random {
             address user = buckets.seen(i);
             uint256 value = buckets.getValueOf(user);
             if (value != 0) {
-                address next = buckets.getNext(value, user);
-                address prev = buckets.getPrev(value, user);
-                assertEq(buckets.getNext(value, prev), user);
-                assertEq(buckets.getPrev(value, next), user);
+                uint256 bucket = LogarithmicBuckets.computeBucket(value);
+                address next = buckets.getNext(bucket, user);
+                address prev = buckets.getPrev(bucket, user);
+                assertEq(buckets.getNext(bucket, prev), user);
+                assertEq(buckets.getPrev(bucket, next), user);
             }
         }
     }