Skip to content

Commit 0dcc11b

Browse files
avniculaeavniculae
committed
feat: add reentrancy locks in spoke
1 parent 6959e32 commit 0dcc11b

File tree

7 files changed

+86
-71
lines changed

7 files changed

+86
-71
lines changed

‎snapshots/NativeTokenGateway.Operations.json‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
{
2-
"borrowNative": "229316",
3-
"repayNative": "168024",
4-
"supplyAsCollateralNative": "160373",
2+
"borrowNative": "229792",
3+
"repayNative": "168492",
4+
"supplyAsCollateralNative": "160402",
55
"supplyNative": "136476",
6-
"withdrawNative: full": "125620",
7-
"withdrawNative: partial": "136825"
6+
"withdrawNative: full": "125996",
7+
"withdrawNative: partial": "137294"
88
}

‎snapshots/SignatureGateway.Operations.json‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
2-
"borrowWithSig": "215605",
3-
"repayWithSig": "188872",
2+
"borrowWithSig": "216081",
3+
"repayWithSig": "189340",
44
"setSelfAsUserPositionManagerWithSig": "75402",
5-
"setUsingAsCollateralWithSig": "85053",
5+
"setUsingAsCollateralWithSig": "85082",
66
"supplyWithSig": "153205",
7-
"updateUserDynamicConfigWithSig": "62769",
8-
"updateUserRiskPremiumWithSig": "61579",
9-
"withdrawWithSig": "131696"
7+
"updateUserDynamicConfigWithSig": "63235",
8+
"updateUserRiskPremiumWithSig": "62045",
9+
"withdrawWithSig": "132071"
1010
}

‎snapshots/Spoke.Getters.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"getUserAccountData: supplies: 0, borrows: 0": "11937",
33
"getUserAccountData: supplies: 1, borrows: 0": "48600",
4-
"getUserAccountData: supplies: 2, borrows: 0": "80378",
4+
"getUserAccountData: supplies: 2, borrows: 0": "80379",
55
"getUserAccountData: supplies: 2, borrows: 1": "100166",
6-
"getUserAccountData: supplies: 2, borrows: 2": "118596"
6+
"getUserAccountData: supplies: 2, borrows: 2": "118595"
77
}

‎snapshots/Spoke.Operations.ZeroRiskPremium.json‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,33 @@
11
{
2-
"borrow: first": "191325",
3-
"borrow: second action, same reserve": "171297",
4-
"liquidationCall (receiveShares): full": "300103",
5-
"liquidationCall (receiveShares): partial": "299821",
6-
"liquidationCall: full": "310468",
7-
"liquidationCall: partial": "310186",
8-
"permitReserve + repay (multicall)": "166029",
2+
"borrow: first": "191801",
3+
"borrow: second action, same reserve": "171773",
4+
"liquidationCall (receiveShares): full": "300568",
5+
"liquidationCall (receiveShares): partial": "300286",
6+
"liquidationCall: full": "310933",
7+
"liquidationCall: partial": "310651",
8+
"permitReserve + repay (multicall)": "166498",
99
"permitReserve + supply (multicall)": "146862",
10-
"permitReserve + supply + enable collateral (multicall)": "160573",
11-
"repay: full": "126094",
12-
"repay: partial": "130983",
10+
"permitReserve + supply + enable collateral (multicall)": "160602",
11+
"repay: full": "126563",
12+
"repay: partial": "131452",
1313
"setUserPositionManagerWithSig: disable": "44846",
1414
"setUserPositionManagerWithSig: enable": "68875",
15-
"supply + enable collateral (multicall)": "140624",
15+
"supply + enable collateral (multicall)": "140653",
1616
"supply: 0 borrows, collateral disabled": "123679",
1717
"supply: 0 borrows, collateral enabled": "106601",
1818
"supply: second action, same reserve": "106579",
19-
"updateUserDynamicConfig: 1 collateral": "73694",
20-
"updateUserDynamicConfig: 2 collaterals": "88551",
21-
"updateUserRiskPremium: 1 borrow": "94804",
22-
"updateUserRiskPremium: 2 borrows": "104619",
23-
"usingAsCollateral: 0 borrows, enable": "58915",
24-
"usingAsCollateral: 1 borrow, disable": "105072",
25-
"usingAsCollateral: 1 borrow, enable": "41803",
26-
"usingAsCollateral: 2 borrows, disable": "126055",
27-
"usingAsCollateral: 2 borrows, enable": "41815",
28-
"withdraw: 0 borrows, full": "128910",
29-
"withdraw: 0 borrows, partial": "133473",
30-
"withdraw: 1 borrow, partial": "161036",
31-
"withdraw: 2 borrows, partial": "174214",
32-
"withdraw: non collateral": "106544"
19+
"updateUserDynamicConfig: 1 collateral": "74160",
20+
"updateUserDynamicConfig: 2 collaterals": "89018",
21+
"updateUserRiskPremium: 1 borrow": "95269",
22+
"updateUserRiskPremium: 2 borrows": "105083",
23+
"usingAsCollateral: 0 borrows, enable": "58944",
24+
"usingAsCollateral: 1 borrow, disable": "105576",
25+
"usingAsCollateral: 1 borrow, enable": "41832",
26+
"usingAsCollateral: 2 borrows, disable": "126558",
27+
"usingAsCollateral: 2 borrows, enable": "41844",
28+
"withdraw: 0 borrows, full": "129379",
29+
"withdraw: 0 borrows, partial": "133942",
30+
"withdraw: 1 borrow, partial": "161504",
31+
"withdraw: 2 borrows, partial": "174681",
32+
"withdraw: non collateral": "107013"
3333
}

‎snapshots/Spoke.Operations.json‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,33 @@
11
{
2-
"borrow: first": "261721",
3-
"borrow: second action, same reserve": "204693",
4-
"liquidationCall (receiveShares): full": "333666",
5-
"liquidationCall (receiveShares): partial": "333384",
6-
"liquidationCall: full": "344031",
7-
"liquidationCall: partial": "343749",
8-
"permitReserve + repay (multicall)": "163273",
2+
"borrow: first": "262197",
3+
"borrow: second action, same reserve": "205169",
4+
"liquidationCall (receiveShares): full": "334131",
5+
"liquidationCall (receiveShares): partial": "333849",
6+
"liquidationCall: full": "344496",
7+
"liquidationCall: partial": "344214",
8+
"permitReserve + repay (multicall)": "163648",
99
"permitReserve + supply (multicall)": "146862",
10-
"permitReserve + supply + enable collateral (multicall)": "160573",
11-
"repay: full": "120256",
12-
"repay: partial": "139545",
10+
"permitReserve + supply + enable collateral (multicall)": "160602",
11+
"repay: full": "120725",
12+
"repay: partial": "140014",
1313
"setUserPositionManagerWithSig: disable": "44846",
1414
"setUserPositionManagerWithSig: enable": "68875",
15-
"supply + enable collateral (multicall)": "140624",
15+
"supply + enable collateral (multicall)": "140653",
1616
"supply: 0 borrows, collateral disabled": "123679",
1717
"supply: 0 borrows, collateral enabled": "106601",
1818
"supply: second action, same reserve": "106579",
19-
"updateUserDynamicConfig: 1 collateral": "73694",
20-
"updateUserDynamicConfig: 2 collaterals": "88551",
21-
"updateUserRiskPremium: 1 borrow": "151080",
22-
"updateUserRiskPremium: 2 borrows": "204276",
23-
"usingAsCollateral: 0 borrows, enable": "58915",
24-
"usingAsCollateral: 1 borrow, disable": "161348",
25-
"usingAsCollateral: 1 borrow, enable": "41803",
26-
"usingAsCollateral: 2 borrows, disable": "233712",
27-
"usingAsCollateral: 2 borrows, enable": "41815",
28-
"withdraw: 0 borrows, full": "128910",
29-
"withdraw: 0 borrows, partial": "133473",
30-
"withdraw: 1 borrow, partial": "214810",
31-
"withdraw: 2 borrows, partial": "259272",
32-
"withdraw: non collateral": "106544"
19+
"updateUserDynamicConfig: 1 collateral": "74160",
20+
"updateUserDynamicConfig: 2 collaterals": "89018",
21+
"updateUserRiskPremium: 1 borrow": "151545",
22+
"updateUserRiskPremium: 2 borrows": "204740",
23+
"usingAsCollateral: 0 borrows, enable": "58944",
24+
"usingAsCollateral: 1 borrow, disable": "161852",
25+
"usingAsCollateral: 1 borrow, enable": "41832",
26+
"usingAsCollateral: 2 borrows, disable": "234215",
27+
"usingAsCollateral: 2 borrows, enable": "41844",
28+
"withdraw: 0 borrows, full": "129379",
29+
"withdraw: 0 borrows, partial": "133942",
30+
"withdraw: 1 borrow, partial": "215278",
31+
"withdraw: 2 borrows, partial": "259739",
32+
"withdraw: non collateral": "107013"
3333
}

‎src/dependencies/openzeppelin/ReentrancyGuardTransient.sol‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,20 @@ abstract contract ReentrancyGuardTransient {
3737
_nonReentrantAfter();
3838
}
3939

40+
/**
41+
* @dev Variant of {nonReentrant} that only enforces the guard if `condition` is true.
42+
* @param condition The condition to check.
43+
*/
44+
modifier nonReentrantConditional(bool condition) {
45+
if (condition) {
46+
_nonReentrantBefore();
47+
_;
48+
_nonReentrantAfter();
49+
} else {
50+
_;
51+
}
52+
}
53+
4054
function _nonReentrantBefore() private {
4155
// On the first call to nonReentrant, REENTRANCY_GUARD_STORAGE.asBoolean().tload() will be false
4256
if (_reentrancyGuardEntered()) {

‎src/spoke/Spoke.sol‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {SafeCast} from 'src/dependencies/openzeppelin/SafeCast.sol';
66
import {SafeERC20, IERC20} from 'src/dependencies/openzeppelin/SafeERC20.sol';
77
import {IERC20Permit} from 'src/dependencies/openzeppelin/IERC20Permit.sol';
88
import {SignatureChecker} from 'src/dependencies/openzeppelin/SignatureChecker.sol';
9+
import {ReentrancyGuardTransient} from 'src/dependencies/openzeppelin/ReentrancyGuardTransient.sol';
910
import {AccessManagedUpgradeable} from 'src/dependencies/openzeppelin-upgradeable/AccessManagedUpgradeable.sol';
1011
import {EIP712} from 'src/dependencies/solady/EIP712.sol';
1112
import {MathUtils} from 'src/libraries/math/MathUtils.sol';
@@ -26,7 +27,7 @@ import {ISpokeBase, ISpoke} from 'src/spoke/interfaces/ISpoke.sol';
2627
/// @author Aave Labs
2728
/// @notice Handles risk configuration & borrowing strategy for reserves and user positions.
2829
/// @dev Each reserve can be associated with a separate Hub.
29-
abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradeable, EIP712 {
30+
abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradeable, ReentrancyGuardTransient, EIP712 {
3031
using SafeCast for *;
3132
using SafeERC20 for IERC20;
3233
using MathUtils for *;
@@ -250,7 +251,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
250251
uint256 reserveId,
251252
uint256 amount,
252253
address onBehalfOf
253-
) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
254+
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
254255
Reserve storage reserve = _getReserve(reserveId);
255256
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
256257
_validateWithdraw(reserve.flags);
@@ -280,7 +281,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
280281
uint256 reserveId,
281282
uint256 amount,
282283
address onBehalfOf
283-
) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
284+
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
284285
Reserve storage reserve = _getReserve(reserveId);
285286
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
286287
PositionStatus storage positionStatus = _positionStatus[onBehalfOf];
@@ -306,7 +307,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
306307
uint256 reserveId,
307308
uint256 amount,
308309
address onBehalfOf
309-
) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
310+
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
310311
Reserve storage reserve = _getReserve(reserveId);
311312
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
312313
_validateRepay(reserve.flags);
@@ -350,7 +351,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
350351
address user,
351352
uint256 debtToCover,
352353
bool receiveShares
353-
) external {
354+
) external nonReentrant {
354355
Reserve storage collateralReserve = _getReserve(collateralReserveId);
355356
Reserve storage debtReserve = _getReserve(debtReserveId);
356357
DynamicReserveConfig storage collateralDynConfig = _dynamicConfig[collateralReserveId][
@@ -404,7 +405,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
404405
uint256 reserveId,
405406
bool usingAsCollateral,
406407
address onBehalfOf
407-
) external onlyPositionManager(onBehalfOf) {
408+
) external nonReentrantConditional(!usingAsCollateral) onlyPositionManager(onBehalfOf) {
408409
_validateSetUsingAsCollateral(_getReserve(reserveId).flags, usingAsCollateral);
409410
PositionStatus storage positionStatus = _positionStatus[onBehalfOf];
410411

@@ -424,7 +425,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
424425
}
425426

426427
/// @inheritdoc ISpoke
427-
function updateUserRiskPremium(address onBehalfOf) external {
428+
function updateUserRiskPremium(address onBehalfOf) external nonReentrant {
428429
if (!_isPositionManager({user: onBehalfOf, manager: msg.sender})) {
429430
_checkCanCall(msg.sender, msg.data);
430431
}
@@ -433,7 +434,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
433434
}
434435

435436
/// @inheritdoc ISpoke
436-
function updateUserDynamicConfig(address onBehalfOf) external {
437+
function updateUserDynamicConfig(address onBehalfOf) external nonReentrant {
437438
if (!_isPositionManager({user: onBehalfOf, manager: msg.sender})) {
438439
_checkCanCall(msg.sender, msg.data);
439440
}

0 commit comments

Comments
 (0)