Skip to content

Commit 53340e3

Browse files
avniculaeavniculae
committed
fix: comments
1 parent 4cb32ed commit 53340e3

9 files changed

+62
-75
lines changed

‎snapshots/AllowancePositionManager.Operations.json‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
{
2-
"AllowancePositionManager: borrowOnBehalfOf": "247328",
3-
"AllowancePositionManager: temporaryApproveWithdraw": "22680",
4-
"AllowancePositionManager: temporaryDelegateCredit": "22768",
5-
"AllowancePositionManager: withdrawOnBehalfOf: full (with temporary allowance)": "56184",
6-
"AllowancePositionManager: withdrawOnBehalfOf: partial (with temporary allowance)": "66984",
72
"approveWithdraw": "46795",
83
"approveWithdrawWithSig": "63006",
9-
"borrowOnBehalfOf": "307050",
4+
"borrowOnBehalfOf": "247328",
105
"delegateCredit": "46784",
116
"delegateCreditWithSig": "63081",
127
"renounceCreditDelegation": "24937",
138
"renounceWithdrawAllowance": "24837",
9+
"temporaryApproveWithdraw": "22680",
10+
"temporaryDelegateCredit": "22768",
1411
"withdrawOnBehalfOf: full": "122006",
15-
"withdrawOnBehalfOf: partial": "132307"
12+
"withdrawOnBehalfOf: full (with temporary allowance)": "56184",
13+
"withdrawOnBehalfOf: partial": "132307",
14+
"withdrawOnBehalfOf: partial (with temporary allowance)": "66984"
1615
}

‎snapshots/PositionManager.Operations.json‎

Lines changed: 0 additions & 29 deletions
This file was deleted.

‎snapshots/SignatureGateway.Operations.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"borrowWithSig": "215605",
2+
"borrowWithSig": "215593",
33
"repayWithSig": "188872",
44
"setSelfAsUserPositionManagerWithSig": "74858",
55
"setUsingAsCollateralWithSig": "85053",

‎snapshots/Spoke.Operations.ZeroRiskPremium.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"repay: full": "126094",
1212
"repay: partial": "130983",
1313
"setUserPositionManagerWithSig: disable": "44846",
14-
"setUserPositionManagerWithSig: enable": "68875",
14+
"setUserPositionManagerWithSig: enable": "68863",
1515
"supply + enable collateral (multicall)": "140624",
1616
"supply: 0 borrows, collateral disabled": "123679",
1717
"supply: 0 borrows, collateral enabled": "106601",

‎snapshots/Spoke.Operations.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"repay: full": "120256",
1212
"repay: partial": "139545",
1313
"setUserPositionManagerWithSig: disable": "44846",
14-
"setUserPositionManagerWithSig: enable": "68875",
14+
"setUserPositionManagerWithSig: enable": "68863",
1515
"supply + enable collateral (multicall)": "140624",
1616
"supply: 0 borrows, collateral disabled": "123679",
1717
"supply: 0 borrows, collateral enabled": "106601",

‎src/position-manager/AllowancePositionManager.sol‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ contract AllowancePositionManager is
8484

8585
/// @inheritdoc IAllowancePositionManager
8686
function temporaryApproveWithdraw(address spender, uint256 reserveId, uint256 amount) external {
87-
_temporaryWithdrawAllowancesSlot(msg.sender, spender, reserveId).tstore(amount);
87+
_temporaryWithdrawAllowancesSlot({owner: msg.sender, spender: spender, reserveId: reserveId})
88+
.tstore(amount);
8889
}
8990

9091
/// @inheritdoc IAllowancePositionManager
@@ -120,7 +121,8 @@ contract AllowancePositionManager is
120121

121122
/// @inheritdoc IAllowancePositionManager
122123
function temporaryDelegateCredit(address spender, uint256 reserveId, uint256 amount) external {
123-
_temporaryDelegateCreditsSlot(msg.sender, spender, reserveId).tstore(amount);
124+
_temporaryDelegateCreditsSlot({owner: msg.sender, spender: spender, reserveId: reserveId})
125+
.tstore(amount);
124126
}
125127

126128
/// @inheritdoc IAllowancePositionManager
@@ -237,16 +239,18 @@ contract AllowancePositionManager is
237239
uint256 reserveId,
238240
uint256 amount
239241
) internal {
240-
uint256 temporaryAllowance = _temporaryWithdrawAllowancesSlot(owner, spender, reserveId)
241-
.tload();
242+
uint256 temporaryAllowance = _temporaryWithdrawAllowancesSlot({
243+
owner: owner,
244+
spender: spender,
245+
reserveId: reserveId
246+
}).tload();
242247
if (temporaryAllowance > 0) {
243248
require(
244249
temporaryAllowance >= amount,
245-
InsufficientWithdrawAllowance(temporaryAllowance, amount)
246-
);
247-
_temporaryWithdrawAllowancesSlot(owner, spender, reserveId).tstore(
248-
temporaryAllowance.uncheckedSub(amount)
250+
InsufficientTemporaryWithdrawAllowance(temporaryAllowance, amount)
249251
);
252+
_temporaryWithdrawAllowancesSlot({owner: owner, spender: spender, reserveId: reserveId})
253+
.tstore(temporaryAllowance.uncheckedSub(amount));
250254
} else {
251255
uint256 allowance = _withdrawAllowances[owner][spender][reserveId];
252256
require(allowance >= amount, InsufficientWithdrawAllowance(allowance, amount));
@@ -261,13 +265,17 @@ contract AllowancePositionManager is
261265
uint256 reserveId,
262266
uint256 amount
263267
) internal {
264-
uint256 temporaryAllowance = _temporaryDelegateCreditsSlot(owner, spender, reserveId).tload();
268+
uint256 temporaryAllowance = _temporaryDelegateCreditsSlot({
269+
owner: owner,
270+
spender: spender,
271+
reserveId: reserveId
272+
}).tload();
265273
if (temporaryAllowance > 0) {
266274
require(
267275
temporaryAllowance >= amount,
268-
InsufficientCreditDelegation(temporaryAllowance, amount)
276+
InsufficientTemporaryCreditDelegation(temporaryAllowance, amount)
269277
);
270-
_temporaryDelegateCreditsSlot(owner, spender, reserveId).tstore(
278+
_temporaryDelegateCreditsSlot({owner: owner, spender: spender, reserveId: reserveId}).tstore(
271279
temporaryAllowance.uncheckedSub(amount)
272280
);
273281
} else {

‎src/position-manager/interfaces/IAllowancePositionManager.sol‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,12 @@ import {IPositionManagerBase} from 'src/position-manager/interfaces/IPositionMan
1111
interface IAllowancePositionManager is IPositionManagerBase {
1212
/// @notice Thrown when the withdraw allowance is insufficient.
1313
error InsufficientWithdrawAllowance(uint256 allowance, uint256 required);
14+
/// @notice Thrown when the temporary withdraw allowance is insufficient.
15+
error InsufficientTemporaryWithdrawAllowance(uint256 allowance, uint256 required);
1416
/// @notice Thrown when the credit delegation allowance is insufficient.
1517
error InsufficientCreditDelegation(uint256 allowance, uint256 required);
18+
/// @notice Thrown when the temporary credit delegation allowance is insufficient.
19+
error InsufficientTemporaryCreditDelegation(uint256 allowance, uint256 required);
1620

1721
/// @notice Emitted when `owner` approves `spender` to withdraw `amount` for `reserveId` on their behalf.
1822
/// @param owner The address of the owner.
@@ -53,6 +57,7 @@ interface IAllowancePositionManager is IPositionManagerBase {
5357
) external;
5458

5559
/// @notice Temporarily approves a spender to withdraw assets from the specified reserve on the spoke.
60+
/// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate.
5661
/// @dev The allowance is discarded after the transaction.
5762
/// @param spender The address of the spender to receive the allowance.
5863
/// @param reserveId The identifier of the reserve.
@@ -74,6 +79,7 @@ interface IAllowancePositionManager is IPositionManagerBase {
7479
) external;
7580

7681
/// @notice Temporarily approves a credit delegation allowance for a spender.
82+
/// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate.
7783
/// @dev The allowance is discarded after the transaction.
7884
/// @param spender The address of the spender to receive the allowance.
7985
/// @param reserveId The identifier of the reserve.
@@ -92,6 +98,7 @@ interface IAllowancePositionManager is IPositionManagerBase {
9298

9399
/// @notice Executes a withdraw on behalf of a user.
94100
/// @dev The caller must have sufficient withdraw allowance from `onBehalfOf`.
101+
/// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate.
95102
/// @dev The caller receives the withdrawn assets.
96103
/// @param reserveId The identifier of the reserve.
97104
/// @param amount The amount to withdraw.
@@ -106,6 +113,7 @@ interface IAllowancePositionManager is IPositionManagerBase {
106113

107114
/// @notice Executes a borrow on behalf of a user.
108115
/// @dev The caller must have sufficient credit delegation allowance from `onBehalfOf`.
116+
/// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate.
109117
/// @dev The caller receives the borrowed assets.
110118
/// @param reserveId The identifier of the reserve.
111119
/// @param amount The amount to borrow.

‎tests/gas/PositionManagers.Operations.gas.t.sol‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -147,20 +147,14 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase {
147147

148148
vm.prank(bob);
149149
positionManager.withdrawOnBehalfOf(_daiReserveId(spoke1), amount, alice);
150-
vm.snapshotGasLastCall(
151-
NAMESPACE,
152-
'AllowancePositionManager: withdrawOnBehalfOf: partial (with temporary allowance)'
153-
);
150+
vm.snapshotGasLastCall(NAMESPACE, 'withdrawOnBehalfOf: partial (with temporary allowance)');
154151

155152
vm.prank(alice);
156153
positionManager.temporaryApproveWithdraw(bob, _daiReserveId(spoke1), UINT256_MAX);
157154

158155
vm.prank(bob);
159156
positionManager.withdrawOnBehalfOf(_daiReserveId(spoke1), UINT256_MAX, alice);
160-
vm.snapshotGasLastCall(
161-
NAMESPACE,
162-
'AllowancePositionManager: withdrawOnBehalfOf: full (with temporary allowance)'
163-
);
157+
vm.snapshotGasLastCall(NAMESPACE, 'withdrawOnBehalfOf: full (with temporary allowance)');
164158
}
165159

166160
function test_borrowOnBehalfOf() public {
@@ -193,7 +187,7 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase {
193187

194188
vm.prank(bob);
195189
positionManager.borrowOnBehalfOf(_daiReserveId(spoke1), borrowAmount, alice);
196-
vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: borrowOnBehalfOf');
190+
vm.snapshotGasLastCall(NAMESPACE, 'borrowOnBehalfOf');
197191
}
198192

199193
function test_approveWithdraw() public {
@@ -232,7 +226,7 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase {
232226

233227
vm.prank(alice);
234228
positionManager.temporaryApproveWithdraw(bob, _daiReserveId(spoke1), amount);
235-
vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: temporaryApproveWithdraw');
229+
vm.snapshotGasLastCall(NAMESPACE, 'temporaryApproveWithdraw');
236230
}
237231

238232
function test_renounceWithdrawAllowance() public {
@@ -282,7 +276,7 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase {
282276

283277
vm.prank(alice);
284278
positionManager.temporaryDelegateCredit(bob, _daiReserveId(spoke1), amount);
285-
vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: temporaryDelegateCredit');
279+
vm.snapshotGasLastCall(NAMESPACE, 'temporaryDelegateCredit');
286280
}
287281

288282
function test_renounceCreditDelegation() public {

‎tests/unit/position-managers/AllowancePositionManager.t.sol‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -440,9 +440,7 @@ contract AllowancePositionManagerTest is SpokeBase {
440440
}
441441

442442
// temporary withdraw allowance takes precedence over stored withdraw allowance, and does not cumulate
443-
function test_withdrawOnBehalfOf_revertsWith_InsufficientWithdrawAllowance_TemporaryAllowanceTakesPrecedence()
444-
public
445-
{
443+
function test_withdrawOnBehalfOf_revertsWith_InsufficientTemporaryWithdrawAllowance() public {
446444
uint256 storedAllowance = 300e18;
447445
_fuzzyApproveWithdraw(alice, alicePk, bob, _daiReserveId(spoke), storedAllowance, 0);
448446

@@ -452,7 +450,7 @@ contract AllowancePositionManagerTest is SpokeBase {
452450

453451
vm.expectRevert(
454452
abi.encodeWithSelector(
455-
IAllowancePositionManager.InsufficientWithdrawAllowance.selector,
453+
IAllowancePositionManager.InsufficientTemporaryWithdrawAllowance.selector,
456454
temporaryAllowance,
457455
amount
458456
)
@@ -463,7 +461,7 @@ contract AllowancePositionManagerTest is SpokeBase {
463461
assertEq(positionManager.withdrawAllowance(alice, bob, _daiReserveId(spoke)), storedAllowance);
464462
}
465463

466-
function test_withdrawOnBehalfOf_fuzz_revertsWith_InsufficientWithdrawAllowance(
464+
function test_withdrawOnBehalfOf_fuzz_revertsWith_InsufficientAllowance(
467465
uint256 approvalAmount,
468466
uint256 approvalType
469467
) public {
@@ -478,11 +476,20 @@ contract AllowancePositionManagerTest is SpokeBase {
478476
onBehalfOf: alice
479477
});
480478

481-
_fuzzyApproveWithdraw(alice, alicePk, bob, _daiReserveId(spoke), approvalAmount, approvalType);
479+
approvalType = _fuzzyApproveWithdraw(
480+
alice,
481+
alicePk,
482+
bob,
483+
_daiReserveId(spoke),
484+
approvalAmount,
485+
approvalType
486+
);
482487

483488
vm.expectRevert(
484489
abi.encodeWithSelector(
485-
IAllowancePositionManager.InsufficientWithdrawAllowance.selector,
490+
(approvalType == 2)
491+
? IAllowancePositionManager.InsufficientTemporaryWithdrawAllowance.selector
492+
: IAllowancePositionManager.InsufficientWithdrawAllowance.selector,
486493
approvalAmount,
487494
amount
488495
)
@@ -734,9 +741,7 @@ contract AllowancePositionManagerTest is SpokeBase {
734741
}
735742

736743
// temporary credit delegation takes precedence over stored credit delegation, and does not cumulate
737-
function test_borrowOnBehalfOf_revertsWith_InsufficientCreditDelegation_temporaryDelegateCreditTakesPrecedence()
738-
public
739-
{
744+
function test_borrowOnBehalfOf_revertsWith_InsufficientTemporaryCreditDelegation() public {
740745
uint256 storedAllowance = 300e18;
741746
_fuzzyDelegateCredit(alice, alicePk, bob, _daiReserveId(spoke), storedAllowance, 0);
742747

@@ -746,7 +751,7 @@ contract AllowancePositionManagerTest is SpokeBase {
746751

747752
vm.expectRevert(
748753
abi.encodeWithSelector(
749-
IAllowancePositionManager.InsufficientCreditDelegation.selector,
754+
IAllowancePositionManager.InsufficientTemporaryCreditDelegation.selector,
750755
temporaryAllowance,
751756
amount
752757
)
@@ -757,7 +762,7 @@ contract AllowancePositionManagerTest is SpokeBase {
757762
assertEq(positionManager.creditDelegation(alice, bob, _daiReserveId(spoke)), storedAllowance);
758763
}
759764

760-
function test_borrowOnBehalfOf_fuzz_revertsWith_InsufficientCreditDelegation(
765+
function test_borrowOnBehalfOf_fuzz_revertsWith_InsufficientAllowance(
761766
uint256 creditDelegationAmount,
762767
uint256 approvalType
763768
) public {
@@ -766,7 +771,7 @@ contract AllowancePositionManagerTest is SpokeBase {
766771
Utils.supplyCollateral(spoke, _daiReserveId(spoke), alice, borrowAmount, alice);
767772
Utils.supplyCollateral(spoke, _daiReserveId(spoke), bob, borrowAmount, bob);
768773

769-
_fuzzyDelegateCredit(
774+
approvalType = _fuzzyDelegateCredit(
770775
alice,
771776
alicePk,
772777
bob,
@@ -777,7 +782,9 @@ contract AllowancePositionManagerTest is SpokeBase {
777782

778783
vm.expectRevert(
779784
abi.encodeWithSelector(
780-
IAllowancePositionManager.InsufficientCreditDelegation.selector,
785+
(approvalType == 2)
786+
? IAllowancePositionManager.InsufficientTemporaryCreditDelegation.selector
787+
: IAllowancePositionManager.InsufficientCreditDelegation.selector,
781788
creditDelegationAmount,
782789
borrowAmount
783790
)

0 commit comments

Comments
 (0)