Skip to content

Commit 27f7e00

Browse files
rft: make PositionStatus iterator behavior mroe consistent (#676)
Co-authored-by: DhairyaSethi <[email protected]>
1 parent 71ca20b commit 27f7e00

File tree

7 files changed

+183
-241
lines changed

7 files changed

+183
-241
lines changed

snapshots/Spoke.Getters.json

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
2-
"getUserAccountData: supplies: 0, borrows: 0": "9329",
3-
"getUserAccountData: supplies: 1, borrows: 0": "44592",
4-
"getUserAccountData: supplies: 2, borrows: 0": "74941",
5-
"getUserAccountData: supplies: 2, borrows: 1": "98305",
6-
"getUserAccountData: supplies: 2, borrows: 2": "120225"
2+
"getUserAccountData: supplies: 0, borrows: 0": "9361",
3+
"getUserAccountData: supplies: 1, borrows: 0": "44652",
4+
"getUserAccountData: supplies: 2, borrows: 0": "75071",
5+
"getUserAccountData: supplies: 2, borrows: 1": "98459",
6+
"getUserAccountData: supplies: 2, borrows: 2": "120403"
77
}

snapshots/Spoke.Operations.json

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,28 @@
11
{
2-
"borrow: first": "257066",
3-
"borrow: second action, same reserve": "220538",
4-
"liquidationCall: full": "326299",
5-
"liquidationCall: partial": "349560",
6-
"permitReserve + repay (multicall)": "279944",
2+
"borrow: first": "257218",
3+
"borrow: second action, same reserve": "220690",
4+
"liquidationCall: full": "326531",
5+
"liquidationCall: partial": "349792",
6+
"permitReserve + repay (multicall)": "280092",
77
"permitReserve + supply (multicall)": "140221",
88
"permitReserve + supply + enable collateral (multicall)": "173706",
9-
"repay: full": "174397",
10-
"repay: partial": "238705",
9+
"repay: full": "174490",
10+
"repay: partial": "238853",
1111
"setUserPositionManagerWithSig: disable": "38977",
1212
"setUserPositionManagerWithSig: enable": "63000",
1313
"supply + enable collateral (multicall)": "152140",
1414
"supply: 0 borrows, collateral disabled": "116034",
1515
"supply: 0 borrows, collateral enabled": "119181",
1616
"supply: 1 borrow": "119170",
1717
"supply: second action, same reserve": "102081",
18-
"updateUserDynamicConfig: 1 collateral": "53152",
19-
"updateUserDynamicConfig: 2 collaterals": "58433",
20-
"updateUserRiskPremium: 1 borrow": "143288",
21-
"updateUserRiskPremium: 2 borrows": "195007",
18+
"updateUserDynamicConfig: 1 collateral": "53205",
19+
"updateUserDynamicConfig: 2 collaterals": "58499",
20+
"updateUserRiskPremium: 1 borrow": "143440",
21+
"updateUserRiskPremium: 2 borrows": "195186",
2222
"usingAsCollateral: 0 borrows, enable": "54026",
23-
"usingAsCollateral: 1 borrow, disable": "153514",
23+
"usingAsCollateral: 1 borrow, disable": "153666",
2424
"usingAsCollateral: 1 borrow, enable": "24786",
25-
"withdraw: 0 borrows, full": "133466",
26-
"withdraw: 0 borrows, partial": "135569",
27-
"withdraw: 1 borrow, partial": "252483"
25+
"withdraw: 0 borrows, full": "133563",
26+
"withdraw: 0 borrows, partial": "135666",
27+
"withdraw: 1 borrow, partial": "252635"
2828
}

src/contracts/Spoke.sol

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -783,19 +783,17 @@ contract Spoke is ISpoke, Multicall, AccessManaged, EIP712 {
783783
address user,
784784
bool refreshConfig
785785
) internal returns (DataTypes.UserAccountData memory userAccountData) {
786-
IAaveOracle aaveOracle = oracle;
787-
uint256 reserveCount = _reserveCount;
788-
789786
DataTypes.PositionStatus storage positionStatus = _positionStatus[user];
787+
788+
IAaveOracle aaveOracle = oracle;
789+
uint256 reserveId = _reserveCount;
790790
KeyValueListInMemory.List memory list = KeyValueListInMemory.init(
791-
positionStatus.collateralCount(reserveCount)
791+
positionStatus.collateralCount(reserveId)
792792
);
793-
794-
uint256 reserveId;
795793
bool borrowing;
796794
bool collateral;
797795
while (true) {
798-
(reserveId, borrowing, collateral) = positionStatus.next(reserveId, reserveCount);
796+
(reserveId, borrowing, collateral) = positionStatus.next(reserveId);
799797
if (reserveId == PositionStatus.NOT_FOUND) break;
800798

801799
DataTypes.UserPosition storage userPosition = _userPositions[user][reserveId];
@@ -844,8 +842,6 @@ contract Spoke is ISpoke, Multicall, AccessManaged, EIP712 {
844842
1
845843
);
846844
}
847-
848-
reserveId = reserveId.uncheckedAdd(1);
849845
}
850846

851847
// at this point avgCollateralFactor is a weighted sum of collateral scaled by collateralFactor
@@ -913,14 +909,10 @@ contract Spoke is ISpoke, Multicall, AccessManaged, EIP712 {
913909
* @param newUserRiskPremium The new risk premium of the user.
914910
*/
915911
function _notifyRiskPremiumUpdate(address user, uint256 newUserRiskPremium) internal {
916-
uint256 reserveCount = _reserveCount;
917912
DataTypes.PositionStatus storage positionStatus = _positionStatus[user];
918913

919-
uint256 reserveId;
920-
while (
921-
(reserveId = positionStatus.nextBorrowing(reserveId, reserveCount)) !=
922-
PositionStatus.NOT_FOUND
923-
) {
914+
uint256 reserveId = _reserveCount;
915+
while ((reserveId = positionStatus.nextBorrowing(reserveId)) != PositionStatus.NOT_FOUND) {
924916
DataTypes.UserPosition storage userPosition = _userPositions[user][reserveId];
925917
uint256 assetId = _reserves[reserveId].assetId;
926918
IHub hub = _reserves[reserveId].hub;
@@ -949,7 +941,6 @@ contract Spoke is ISpoke, Multicall, AccessManaged, EIP712 {
949941

950942
hub.refreshPremium(assetId, premiumDelta);
951943
emit RefreshPremiumDebt(reserveId, user, premiumDelta);
952-
reserveId = reserveId.uncheckedAdd(1);
953944
}
954945
emit UserRiskPremiumUpdate(user, newUserRiskPremium);
955946
}
@@ -961,13 +952,9 @@ contract Spoke is ISpoke, Multicall, AccessManaged, EIP712 {
961952
*/
962953
function _reportDeficit(address user) internal {
963954
DataTypes.PositionStatus storage positionStatus = _positionStatus[user];
964-
uint256 reserveCount = _reserveCount;
965-
uint256 reserveId;
955+
uint256 reserveId = _reserveCount;
966956

967-
while (
968-
(reserveId = positionStatus.nextBorrowing(reserveId, reserveCount)) !=
969-
PositionStatus.NOT_FOUND
970-
) {
957+
while ((reserveId = positionStatus.nextBorrowing(reserveId)) != PositionStatus.NOT_FOUND) {
971958
DataTypes.UserPosition storage userPosition = _userPositions[user][reserveId];
972959
DataTypes.Reserve storage reserve = _reserves[reserveId];
973960
// validation should already have occurred during liquidation
@@ -995,23 +982,15 @@ contract Spoke is ISpoke, Multicall, AccessManaged, EIP712 {
995982
// newUserRiskPremium is 0 due to no collateral remaining
996983
// non-zero deficit means user ends up with zero total debt
997984
positionStatus.setBorrowing(reserveId, false);
998-
999-
reserveId = reserveId.uncheckedAdd(1);
1000985
}
1001986
emit UserRiskPremiumUpdate(user, 0);
1002987
}
1003988

1004989
function _refreshDynamicConfig(address user) internal {
1005-
uint256 reserveCount = _reserveCount;
1006-
uint256 reserveId;
990+
uint256 reserveId = _reserveCount;
1007991
DataTypes.PositionStatus storage positionStatus = _positionStatus[user];
1008-
while (
1009-
(reserveId = positionStatus.nextCollateral(reserveId, reserveCount)) !=
1010-
PositionStatus.NOT_FOUND
1011-
) {
992+
while ((reserveId = positionStatus.nextCollateral(reserveId)) != PositionStatus.NOT_FOUND) {
1012993
_userPositions[user][reserveId].configKey = _reserves[reserveId].dynamicConfigKey;
1013-
1014-
reserveId = reserveId.uncheckedAdd(1);
1015994
}
1016995
emit RefreshAllUserDynamicConfig(user);
1017996
}

src/dependencies/solady/LibBit.sol

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,27 +20,26 @@ library LibBit {
2020
}
2121
}
2222

23-
/// @dev Find first set.
24-
/// Returns the index of the least significant bit of `x`,
23+
/// @dev Find last set.
24+
/// Returns the index of the most significant bit of `x`,
2525
/// counting from the least significant bit position.
2626
/// If `x` is zero, returns 256.
27-
/// Equivalent to `ctz` (count trailing zeros), which gives
28-
/// the number of zeros following the least significant one bit.
29-
function ffs(uint256 x) internal pure returns (uint256 r) {
27+
function fls(uint256 x) internal pure returns (uint256 r) {
3028
/// @solidity memory-safe-assembly
3129
assembly {
32-
// Isolate the least significant bit.
33-
x := and(x, add(not(x), 1))
34-
// For the upper 3 bits of the result, use a De Bruijn-like lookup.
35-
// Credit to adhusson: https://blog.adhusson.com/cheap-find-first-set-evm/
36-
// prettier-ignore
37-
r := shl(5, shr(252, shl(shl(2, shr(250, mul(x,
38-
0xb6db6db6ddddddddd34d34d349249249210842108c6318c639ce739cffffffff))),
39-
0x8040405543005266443200005020610674053026020000107506200176117077)))
40-
// For the lower 5 bits of the result, use a De Bruijn lookup.
41-
// prettier-ignore
42-
r := or(r, byte(and(div(0xd76453e0, shr(r, x)), 0x1f),
43-
0x001f0d1e100c1d070f090b19131c1706010e11080a1a141802121b1503160405))
30+
r := or(shl(8, iszero(x)), shl(7, lt(0xffffffffffffffffffffffffffffffff, x)))
31+
r := or(r, shl(6, lt(0xffffffffffffffff, shr(r, x))))
32+
r := or(r, shl(5, lt(0xffffffff, shr(r, x))))
33+
r := or(r, shl(4, lt(0xffff, shr(r, x))))
34+
r := or(r, shl(3, lt(0xff, shr(r, x))))
35+
// forgefmt: disable-next-item
36+
r := or(
37+
r,
38+
byte(
39+
and(0x1f, shr(shr(r, x), 0x8421084210842108cc6318c6db6d54be)),
40+
0x0706060506020504060203020504030106050205030304010505030400000000
41+
)
42+
)
4443
}
4544
}
4645
}

src/libraries/configuration/PositionStatus.sol

Lines changed: 54 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -129,29 +129,27 @@ library PositionStatus {
129129
}
130130

131131
/**
132-
* @dev Returns the next reserveId that is borrowing or using as collateral.
133-
* @dev Returns NOT_FOUND if no such reserveId exists, starts searching from `startReserveId`.
134-
* @dev Does not disregard potential dirty bits set after `reserveCount` in it's bucket.
132+
* @dev Finds the previous borrowing or collateralized reserve strictly before `fromReserveId`.
133+
* @dev The search starts at `fromReserveId` (exclusive) and scans backward across buckets.
134+
* @dev Returns `NOT_FOUND` if no borrowing or collateralized reserve exists before the bound.
135+
* @dev Ignores dirty bits beyond the configured `reserveCount` within the current bucket.
135136
* @param self The configuration object.
136-
* @param startReserveId The reserveId to start searching from.
137-
* @param reserveCount The current reserveCount, to avoid reading uninitialized buckets.
137+
* @param fromReserveId The reserveId to start searching from.
138138
* @return reserveId The next reserveId that is borrowing or using as collateral.
139139
* @return borrowing True if the next reserveId is borrowing, false otherwise.
140140
* @return collateral True if the next reserveId is using as collateral, false otherwise.
141141
*/
142142
function next(
143143
DataTypes.PositionStatus storage self,
144-
uint256 startReserveId,
145-
uint256 reserveCount
144+
uint256 fromReserveId
146145
) internal view returns (uint256, bool, bool) {
147146
unchecked {
148-
uint256 endBucket = reserveCount.bucketId();
149-
uint256 bucket = startReserveId.bucketId();
147+
uint256 bucket = fromReserveId.bucketId();
150148
uint256 map = self.map[bucket];
151-
uint256 setBitId = map.isolateFrom(startReserveId).ffs();
152-
while (setBitId == 256 && bucket != endBucket) {
153-
map = self.map[++bucket];
154-
setBitId = map.ffs();
149+
uint256 setBitId = map.isolateUntil(fromReserveId).fls();
150+
while (setBitId == 256 && bucket != 0) {
151+
map = self.map[--bucket];
152+
setBitId = map.fls();
155153
}
156154
if (setBitId == 256) {
157155
return (NOT_FOUND, false, false);
@@ -163,50 +161,46 @@ library PositionStatus {
163161
}
164162

165163
/**
166-
* @dev Returns the next reserveId that is borrowing.
167-
* @dev Returns NOT_FOUND if no such reserveId exists, starts searching from `startReserveId`.
168-
* @dev Does not disregard potential dirty bits set after `reserveCount` in it's bucket.
169-
* @param self The configuration object.
170-
* @param startReserveId The reserveId to start searching from.
171-
* @param reserveCount The current reserveCount, to avoid reading uninitialized buckets.
172-
* @return reserveId The next reserveId that is borrowing.
164+
* @dev Finds the previous borrowing reserve strictly before `fromReserveId`.
165+
* @dev The search starts at `fromReserveId` (exclusive) and scans backward across buckets.
166+
* @dev Returns `NOT_FOUND` if no borrowing reserve exists before the bound.
167+
* @dev Ignores dirty bits beyond the configured `reserveCount` within the current bucket.
168+
* @param self The position status storing reserves bitmap.
169+
* @param fromReserveId The exclusive upper bound to start from (this reserveId is not considered).
170+
* @return reserveId The previous borrowing reserveId, or `NOT_FOUND` if none is found.
173171
*/
174172
function nextBorrowing(
175173
DataTypes.PositionStatus storage self,
176-
uint256 startReserveId,
177-
uint256 reserveCount
174+
uint256 fromReserveId
178175
) internal view returns (uint256 reserveId) {
179176
unchecked {
180-
uint256 endBucket = reserveCount.bucketId();
181-
uint256 bucket = startReserveId.bucketId();
182-
uint256 setBitId = self.map[bucket].isolateBorrowingFrom(startReserveId).ffs();
183-
while (setBitId == 256 && bucket != endBucket) {
184-
setBitId = self.map[++bucket].isolateBorrowing().ffs();
177+
uint256 bucket = fromReserveId.bucketId();
178+
uint256 setBitId = self.map[bucket].isolateBorrowingUntil(fromReserveId).fls();
179+
while (setBitId == 256 && bucket != 0) {
180+
setBitId = self.map[--bucket].isolateBorrowing().fls();
185181
}
186182
return setBitId == 256 ? NOT_FOUND : setBitId.fromBitId(bucket);
187183
}
188184
}
189185

190186
/**
191-
* @dev Returns the next reserveId that is using as collateral.
192-
* @dev Returns NOT_FOUND if no such reserveId exists, starts searching from `startReserveId`.
193-
* @dev Does not disregard potential dirty bits set after `reserveCount` in it's bucket.
194-
* @param self The configuration object.
195-
* @param startReserveId The reserveId to start searching from.
196-
* @param reserveCount The current reserveCount, to avoid reading uninitialized buckets.
197-
* @return reserveId The next reserveId that is using as collateral.
187+
* @dev Finds the previous collateralized reserve strictly before `fromReserveId`.
188+
* @dev The search starts at `fromReserveId` (exclusive) and scans backward across buckets.
189+
* @dev Returns `NOT_FOUND` if no collateralized reserve exists before the bound.
190+
* @dev Ignores dirty bits beyond the configured `reserveCount` within the current bucket.
191+
* @param self The position status storing reserves bitmap.
192+
* @param fromReserveId The exclusive upper bound to start from (this reserveId is not considered).
193+
* @return reserveId The previous collateralized reserveId, or `NOT_FOUND` if none is found.
198194
*/
199195
function nextCollateral(
200196
DataTypes.PositionStatus storage self,
201-
uint256 startReserveId,
202-
uint256 reserveCount
197+
uint256 fromReserveId
203198
) internal view returns (uint256 reserveId) {
204199
unchecked {
205-
uint256 endBucket = reserveCount.bucketId();
206-
uint256 bucket = startReserveId.bucketId();
207-
uint256 setBitId = self.map[bucket].isolateCollateralFrom(startReserveId).ffs();
208-
while (setBitId == 256 && bucket != endBucket) {
209-
setBitId = self.map[++bucket].isolateCollateral().ffs();
200+
uint256 bucket = fromReserveId.bucketId();
201+
uint256 setBitId = self.map[bucket].isolateCollateralUntil(fromReserveId).fls();
202+
while (setBitId == 256 && bucket != 0) {
203+
setBitId = self.map[--bucket].isolateCollateral().fls();
210204
}
211205
return setBitId == 256 ? NOT_FOUND : setBitId.fromBitId(bucket);
212206
}
@@ -253,25 +247,31 @@ library PositionStatus {
253247
}
254248

255249
/**
256-
* @dev Isolates the borrowing bits from word, disregarding bits before `reserveId`.
250+
* @dev Isolates borrowing bits up to the given `reserveCount`, clearing all later reserves.
251+
* @param word The 256-bit value encoding reserves configuration.
252+
* @param reserveCount The number of reserves (2 bits each) to include.
253+
* @return ret The portion of word containing borrowing bits from the first reserve up to `reserveCount`.
257254
*/
258-
function isolateBorrowingFrom(
255+
function isolateBorrowingUntil(
259256
uint256 word,
260-
uint256 reserveId
257+
uint256 reserveCount
261258
) internal pure returns (uint256 ret) {
262-
// ret = word & (BORROWING_MASK << ((reserveId % 128) << 1));
259+
// ret = word & (BORROWING_MASK >> (256 - ((reserveCount % 128) << 1)));
263260
assembly ('memory-safe') {
264-
ret := and(word, shl(shl(1, mod(reserveId, 128)), BORROWING_MASK))
261+
ret := and(word, shr(sub(256, shl(1, mod(reserveCount, 128))), BORROWING_MASK))
265262
}
266263
}
267264

268265
/**
269-
* @dev Isolates the bits from word, disregarding bits before `reserveId`.
266+
* @dev Isolates bits up to the given `reserveCount`, clearing all later reserves.
267+
* @param word The 256-bit value encoding reserves configuration.
268+
* @param reserveCount The number of reserves (2 bits each) to include.
269+
* @return ret The portion of word containing bits from the first reserve up to `reserveCount`.
270270
*/
271-
function isolateFrom(uint256 word, uint256 reserveId) internal pure returns (uint256 ret) {
272-
// ret = word & (type(uint256).max << ((reserveId % 128) << 1));
271+
function isolateUntil(uint256 word, uint256 reserveCount) internal pure returns (uint256 ret) {
272+
// ret = word & (type(uint256).max >> (256 - ((reserveCount % 128) << 1)));
273273
assembly ('memory-safe') {
274-
ret := and(word, shl(shl(1, mod(reserveId, 128)), not(0)))
274+
ret := and(word, shr(sub(256, shl(1, mod(reserveCount, 128))), not(0)))
275275
}
276276
}
277277

@@ -285,20 +285,10 @@ library PositionStatus {
285285
}
286286

287287
/**
288-
* @dev Isolates the bits from word, disregarding bits before `reserveId`.
289-
*/
290-
function isolateCollateralFrom(
291-
uint256 word,
292-
uint256 reserveId
293-
) internal pure returns (uint256 ret) {
294-
// ret = word & (COLLATERAL_MASK << ((reserveId % 128) << 1));
295-
assembly ('memory-safe') {
296-
ret := and(word, shl(shl(1, mod(reserveId, 128)), COLLATERAL_MASK))
297-
}
298-
}
299-
300-
/**
301-
* @dev Isolates the bits from word, disregarding bits after `reserveCount`.
288+
* @dev Isolates collateral bits up to the given `reserveCount`, clearing all later reserves.
289+
* @param word The 256-bit value encoding reserves configuration.
290+
* @param reserveCount The number of reserves (2 bits each) to include.
291+
* @return ret The portion of word containing collateral bits from the first reserve up to `reserveCount`.
302292
*/
303293
function isolateCollateralUntil(
304294
uint256 word,

0 commit comments

Comments
 (0)