From a443ac9c1d4340ddd4dcb0df274ebaa8b5702bf4 Mon Sep 17 00:00:00 2001 From: Alexandru Niculae <43644109+avniculae@users.noreply.github.com> Date: Mon, 15 Dec 2025 14:47:33 +0200 Subject: [PATCH 1/8] feat: temporary approvals in AllowancePositionManager --- snapshots/PositionManager.Operations.json | 6 +- .../AllowancePositionManager.sol | 110 ++++++- .../interfaces/IAllowancePositionManager.sol | 18 ++ .../mocks/AllowancePositionManagerWrapper.sol | 28 ++ .../AllowancePositionManager.t.sol | 305 +++++++++++++++--- 5 files changed, 419 insertions(+), 48 deletions(-) create mode 100644 tests/mocks/AllowancePositionManagerWrapper.sol diff --git a/snapshots/PositionManager.Operations.json b/snapshots/PositionManager.Operations.json index 14b6fc6da..72bf658c5 100644 --- a/snapshots/PositionManager.Operations.json +++ b/snapshots/PositionManager.Operations.json @@ -1,7 +1,7 @@ { - "AllowancePositionManager - borrowOnBehalfOf": "306478", - "AllowancePositionManager - withdrawOnBehalfOf: full": "121565", - "AllowancePositionManager - withdrawOnBehalfOf: partial": "131756", + "AllowancePositionManager - borrowOnBehalfOf": "307050", + "AllowancePositionManager - withdrawOnBehalfOf: full": "122024", + "AllowancePositionManager - withdrawOnBehalfOf: partial": "132329", "Common - setSelfAsUserPositionManagerWithSig": "72588", "SupplyRepayPositionManager - repayOnBehalfOf": "167131", "SupplyRepayPositionManager - supplyOnBehalfOf": "135803" diff --git a/src/position-manager/AllowancePositionManager.sol b/src/position-manager/AllowancePositionManager.sol index e382fb05b..e97f49705 100644 --- a/src/position-manager/AllowancePositionManager.sol +++ b/src/position-manager/AllowancePositionManager.sol @@ -4,6 +4,8 @@ pragma solidity 0.8.28; import {SignatureChecker} from 'src/dependencies/openzeppelin/SignatureChecker.sol'; import {SafeERC20, IERC20} from 'src/dependencies/openzeppelin/SafeERC20.sol'; +import {SlotDerivation} from 'src/dependencies/openzeppelin/SlotDerivation.sol'; +import {TransientSlot} from 'src/dependencies/openzeppelin/TransientSlot.sol'; import {EIP712} from 'src/dependencies/solady/EIP712.sol'; import {MathUtils} from 'src/libraries/math/MathUtils.sol'; import {NoncesKeyed} from 'src/utils/NoncesKeyed.sol'; @@ -24,15 +26,27 @@ contract AllowancePositionManager is using SafeERC20 for IERC20; using EIP712Hash for *; using MathUtils for uint256; + using SlotDerivation for bytes32; + using TransientSlot for *; /// @notice Mapping of withdraw allowances. mapping(address owner => mapping(address spender => mapping(uint256 reserveId => uint256 amount))) private _withdrawAllowances; + /// @notice Slot for the temporary withdraw allowances. + /// @dev keccak256('temporary.withdrawAllowances') + bytes32 private constant _TEMPORARY_WITHDRAW_ALLOWANCES_SLOT = + 0x1c6a61279a13a86a789311ddf30aee38e2f4a9f6c4aad1ff4a2e75a4018e68c3; + /// @notice Mapping of credit delegations. mapping(address owner => mapping(address spender => mapping(uint256 reserveId => uint256 amount))) private _creditDelegations; + /// @notice Slot for the temporary credit delegations. + /// @dev keccak256('temporary.creditDelegations') + bytes32 private constant _TEMPORARY_CREDIT_DELEGATIONS_SLOT = + 0xcd470af8670f5baa744a0341af8a2e3f5d7ca086178908432a5cfaf39cb9299d; + /// @dev Constructor. /// @param spoke_ The address of the spoke contract. constructor(address spoke_) PositionManagerBase(spoke_) {} @@ -58,6 +72,11 @@ contract AllowancePositionManager is emit WithdrawApproval(user, params.spender, params.reserveId, params.amount); } + /// @inheritdoc IAllowancePositionManager + function temporaryApproveWithdraw(address spender, uint256 reserveId, uint256 amount) external { + _temporaryWithdrawAllowancesSlot(msg.sender, spender, reserveId).tstore(amount); + } + /// @inheritdoc IAllowancePositionManager function approveCreditDelegation(address spender, uint256 reserveId, uint256 amount) external { _creditDelegations[msg.sender][spender][reserveId] = amount; @@ -79,6 +98,15 @@ contract AllowancePositionManager is emit CreditDelegation(user, params.spender, params.reserveId, params.amount); } + /// @inheritdoc IAllowancePositionManager + function temporaryApproveCreditDelegation( + address spender, + uint256 reserveId, + uint256 amount + ) external { + _temporaryCreditDelegationsSlot(msg.sender, spender, reserveId).tstore(amount); + } + /// @inheritdoc IAllowancePositionManager function renounceWithdrawAllowance(address owner, uint256 reserveId) external { _withdrawAllowances[owner][msg.sender][reserveId] = 0; @@ -98,9 +126,7 @@ contract AllowancePositionManager is address onBehalfOf ) external returns (uint256, uint256) { require(amount > 0, InvalidAmount()); - uint256 currentAllowance = _withdrawAllowances[onBehalfOf][msg.sender][reserveId]; - require(currentAllowance >= amount, InsufficientWithdrawAllowance(currentAllowance, amount)); - _withdrawAllowances[onBehalfOf][msg.sender][reserveId] = currentAllowance.uncheckedSub(amount); + _spendWithdrawAllowance(onBehalfOf, msg.sender, reserveId, amount); IERC20 asset = _getReserveUnderlying(reserveId); (uint256 withdrawnShares, uint256 withdrawnAmount) = ISpokeBase(SPOKE).withdraw( @@ -120,9 +146,7 @@ contract AllowancePositionManager is address onBehalfOf ) external returns (uint256, uint256) { require(amount > 0, InvalidAmount()); - uint256 currentAllowance = _creditDelegations[onBehalfOf][msg.sender][reserveId]; - require(currentAllowance >= amount, InsufficientCreditDelegation(currentAllowance, amount)); - _creditDelegations[onBehalfOf][msg.sender][reserveId] = currentAllowance.uncheckedSub(amount); + _spendCreditDelegation(onBehalfOf, msg.sender, reserveId, amount); IERC20 asset = _getReserveUnderlying(reserveId); (uint256 borrowedShares, uint256 borrowedAmount) = ISpokeBase(SPOKE).borrow( @@ -171,4 +195,78 @@ contract AllowancePositionManager is function _domainNameAndVersion() internal pure override returns (string memory, string memory) { return ('AllowancePositionManager', '1'); } + + /// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate. + function _spendWithdrawAllowance( + address onBehalfOf, + address spender, + uint256 reserveId, + uint256 amount + ) internal { + uint256 temporaryAllowance = _temporaryWithdrawAllowancesSlot(onBehalfOf, spender, reserveId) + .tload(); + if (temporaryAllowance > 0) { + require( + temporaryAllowance >= amount, + InsufficientWithdrawAllowance(temporaryAllowance, amount) + ); + _temporaryWithdrawAllowancesSlot(onBehalfOf, spender, reserveId).tstore( + temporaryAllowance.uncheckedSub(amount) + ); + } else { + uint256 allowance = _withdrawAllowances[onBehalfOf][spender][reserveId]; + require(allowance >= amount, InsufficientWithdrawAllowance(allowance, amount)); + _withdrawAllowances[onBehalfOf][spender][reserveId] = allowance.uncheckedSub(amount); + } + } + + /// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate. + function _spendCreditDelegation( + address onBehalfOf, + address spender, + uint256 reserveId, + uint256 amount + ) internal { + uint256 temporaryAllowance = _temporaryCreditDelegationsSlot(onBehalfOf, spender, reserveId) + .tload(); + if (temporaryAllowance > 0) { + require( + temporaryAllowance >= amount, + InsufficientCreditDelegation(temporaryAllowance, amount) + ); + _temporaryCreditDelegationsSlot(onBehalfOf, spender, reserveId).tstore( + temporaryAllowance.uncheckedSub(amount) + ); + } else { + uint256 allowance = _creditDelegations[onBehalfOf][spender][reserveId]; + require(allowance >= amount, InsufficientCreditDelegation(allowance, amount)); + _creditDelegations[onBehalfOf][spender][reserveId] = allowance.uncheckedSub(amount); + } + } + + function _temporaryWithdrawAllowancesSlot( + address owner, + address spender, + uint256 reserveId + ) internal pure returns (TransientSlot.Uint256Slot) { + return + _TEMPORARY_WITHDRAW_ALLOWANCES_SLOT + .deriveMapping(owner) + .deriveMapping(spender) + .deriveMapping(reserveId) + .asUint256(); + } + + function _temporaryCreditDelegationsSlot( + address owner, + address spender, + uint256 reserveId + ) internal pure returns (TransientSlot.Uint256Slot) { + return + _TEMPORARY_CREDIT_DELEGATIONS_SLOT + .deriveMapping(owner) + .deriveMapping(spender) + .deriveMapping(reserveId) + .asUint256(); + } } diff --git a/src/position-manager/interfaces/IAllowancePositionManager.sol b/src/position-manager/interfaces/IAllowancePositionManager.sol index 762c455b5..5c5185e7a 100644 --- a/src/position-manager/interfaces/IAllowancePositionManager.sol +++ b/src/position-manager/interfaces/IAllowancePositionManager.sol @@ -44,6 +44,13 @@ interface IAllowancePositionManager is IPositionManagerBase { bytes calldata signature ) external; + /// @notice Temporarily approves a spender to withdraw assets from the specified reserve on the spoke. + /// @dev The allowance is discarded after the transaction. + /// @param spender The address of the spender to receive the allowance. + /// @param reserveId The identifier of the reserve. + /// @param amount The amount of allowance. + function temporaryApproveWithdraw(address spender, uint256 reserveId, uint256 amount) external; + /// @notice Approves a credit delegation allowance for a spender. /// @param spender The address of the spender to receive the allowance. /// @param reserveId The identifier of the reserve. @@ -58,6 +65,17 @@ interface IAllowancePositionManager is IPositionManagerBase { bytes calldata signature ) external; + /// @notice Temporarily approves a credit delegation allowance for a spender. + /// @dev The allowance is discarded after the transaction. + /// @param spender The address of the spender to receive the allowance. + /// @param reserveId The identifier of the reserve. + /// @param amount The amount of allowance. + function temporaryApproveCreditDelegation( + address spender, + uint256 reserveId, + uint256 amount + ) external; + /// @notice Renounces the withdraw allowance given by the owner. /// @param owner The address of the owner. /// @param reserveId The identifier of the reserve. diff --git a/tests/mocks/AllowancePositionManagerWrapper.sol b/tests/mocks/AllowancePositionManagerWrapper.sol new file mode 100644 index 000000000..350db22d6 --- /dev/null +++ b/tests/mocks/AllowancePositionManagerWrapper.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: UNLICENSED +// Copyright (c) 2025 Aave Labs +pragma solidity ^0.8.0; + +import {TransientSlot} from 'src/dependencies/openzeppelin/TransientSlot.sol'; +import {AllowancePositionManager} from 'src/position-manager/AllowancePositionManager.sol'; + +contract AllowancePositionManagerWrapper is AllowancePositionManager { + using TransientSlot for *; + + constructor(address spoke_) AllowancePositionManager(spoke_) {} + + function temporaryWithdrawAllowance( + address owner, + address spender, + uint256 reserveId + ) external view returns (uint256) { + return _temporaryWithdrawAllowancesSlot(owner, spender, reserveId).tload(); + } + + function temporaryCreditDelegation( + address owner, + address spender, + uint256 reserveId + ) external view returns (uint256) { + return _temporaryCreditDelegationsSlot(owner, spender, reserveId).tload(); + } +} diff --git a/tests/unit/position-managers/AllowancePositionManager.t.sol b/tests/unit/position-managers/AllowancePositionManager.t.sol index 3db93a58e..599689b87 100644 --- a/tests/unit/position-managers/AllowancePositionManager.t.sol +++ b/tests/unit/position-managers/AllowancePositionManager.t.sol @@ -2,21 +2,27 @@ // Copyright (c) 2025 Aave Labs pragma solidity ^0.8.0; +import {AllowancePositionManagerWrapper} from 'tests/mocks/AllowancePositionManagerWrapper.sol'; import 'tests/unit/Spoke/SpokeBase.t.sol'; contract AllowancePositionManagerTest is SpokeBase { ISpoke public spoke; - AllowancePositionManager public positionManager; + AllowancePositionManagerWrapper public positionManager; TestReturnValues public returnValues; uint256 public alicePk; + bytes32 private constant _TEMPORARY_WITHDRAW_ALLOWANCES_SLOT = + 0x1c6a61279a13a86a789311ddf30aee38e2f4a9f6c4aad1ff4a2e75a4018e68c3; + bytes32 private constant _TEMPORARY_CREDIT_DELEGATIONS_SLOT = + 0xcd470af8670f5baa744a0341af8a2e3f5d7ca086178908432a5cfaf39cb9299d; + function setUp() public virtual override { deployFixtures(); initEnvironment(); spoke = spoke1; (alice, alicePk) = makeAddrAndKey('alice'); - positionManager = new AllowancePositionManager(address(spoke)); + positionManager = new AllowancePositionManagerWrapper(address(spoke)); vm.prank(SPOKE_ADMIN); spoke.updatePositionManager(address(positionManager), true); @@ -79,7 +85,7 @@ contract AllowancePositionManagerTest is SpokeBase { function test_approveWithdraw_fuzz(address spender, uint256 reserveId, uint256 amount) public { vm.assume(spender != address(0)); reserveId = bound(reserveId, 0, spoke.getReserveCount() - 1); - amount = bound(amount, 1, mintAmount_DAI); + amount = bound(amount, 1, MAX_SUPPLY_AMOUNT); vm.expectEmit(address(positionManager)); emit IAllowancePositionManager.WithdrawApproval(alice, spender, reserveId, amount); @@ -96,7 +102,7 @@ contract AllowancePositionManagerTest is SpokeBase { ) public { vm.assume(spender != address(0)); reserveId = bound(reserveId, 0, spoke.getReserveCount() - 1); - amount = bound(amount, 1, mintAmount_DAI); + amount = bound(amount, 1, MAX_SUPPLY_AMOUNT); EIP712Types.WithdrawPermit memory p = _withdrawPermitData( spender, @@ -146,7 +152,7 @@ contract AllowancePositionManagerTest is SpokeBase { positionManager.approveWithdrawWithSig(p, signature); } - function test_approveWithdrawWithSig_revertsWith_InvalidAccountNonce(bytes32) public { + function test_approveWithdrawWithSig_fuzz_revertsWith_InvalidAccountNonce(bytes32) public { EIP712Types.WithdrawPermit memory p = _withdrawPermitData( vm.randomAddress(), alice, @@ -167,7 +173,7 @@ contract AllowancePositionManagerTest is SpokeBase { function test_renounceWithdrawAllowance_fuzz(uint256 initialAllowance) public { uint256 reserveId = _randomReserveId(spoke); - initialAllowance = bound(initialAllowance, 1, mintAmount_DAI); + initialAllowance = bound(initialAllowance, 1, MAX_SUPPLY_AMOUNT); vm.prank(alice); positionManager.approveWithdraw(bob, reserveId, initialAllowance); @@ -180,11 +186,43 @@ contract AllowancePositionManagerTest is SpokeBase { assertEq(positionManager.withdrawAllowance(alice, bob, reserveId), 0); } + function test_temporaryApproveWithdraw_fuzz( + address spender, + uint256 reserveId, + uint256 amount + ) public { + vm.assume(spender != address(0)); + reserveId = bound(reserveId, 0, spoke.getReserveCount() - 1); + amount = bound(amount, 1, MAX_SUPPLY_AMOUNT); + + vm.expectEmit(address(positionManager), 0); + vm.prank(alice); + positionManager.temporaryApproveWithdraw(spender, reserveId, amount); + + assertEq(positionManager.temporaryWithdrawAllowance(alice, spender, reserveId), amount); + } + + /// forge-config: default.isolate = true + function test_temporaryApproveWithdraw_TransientStorage() public { + // make sure transient storage is used for temporary withdraw allowances + vm.prank(alice); + positionManager.temporaryApproveWithdraw(bob, _daiReserveId(spoke), 100e18); + assertEq(positionManager.temporaryWithdrawAllowance(alice, bob, _daiReserveId(spoke)), 0); + } + function test_withdrawOnBehalfOf() public { - test_withdrawOnBehalfOf_fuzz(100e18); + test_withdrawOnBehalfOf_fuzz(100e18, 0); + } + + function test_withdrawOnBehalfOf_TemporaryAllowanceTakesPrecedence() public { + uint256 storedAllowance = 300e18; + _fuzzyApproveWithdraw(alice, alicePk, bob, _daiReserveId(spoke), storedAllowance, 0); + test_withdrawOnBehalfOf_fuzz(100e18, 2); + // this check is also performed in test_withdrawOnBehalfOf_fuzz, duplicating in case of future changes + assertEq(positionManager.withdrawAllowance(alice, bob, _daiReserveId(spoke)), storedAllowance); } - function test_withdrawOnBehalfOf_fuzz(uint256 amount) public { + function test_withdrawOnBehalfOf_fuzz(uint256 amount, uint256 approvalType) public { amount = bound(amount, 1, mintAmount_DAI); Utils.supply({ @@ -196,8 +234,12 @@ contract AllowancePositionManagerTest is SpokeBase { }); uint256 expectedSupplyShares = hub1.previewAddByAssets(daiAssetId, mintAmount_DAI); - vm.prank(alice); - positionManager.approveWithdraw(bob, _daiReserveId(spoke), amount); + uint256 withdrawAllowanceBefore = positionManager.withdrawAllowance( + alice, + bob, + _daiReserveId(spoke) + ); + _fuzzyApproveWithdraw(alice, alicePk, bob, _daiReserveId(spoke), amount, approvalType); uint256 prevUserBalance = tokenList.dai.balanceOf(alice); uint256 prevCallerBalance = tokenList.dai.balanceOf(bob); @@ -233,10 +275,16 @@ contract AllowancePositionManagerTest is SpokeBase { assertEq(tokenList.dai.balanceOf(address(hub1)), prevHubBalance - amount); assertEq(tokenList.dai.balanceOf(address(positionManager)), 0); assertEq(tokenList.dai.allowance(address(positionManager), address(hub1)), 0); - assertEq(positionManager.withdrawAllowance(alice, bob, _daiReserveId(spoke)), 0); + assertEq( + positionManager.withdrawAllowance(alice, bob, _daiReserveId(spoke)), + (approvalType < 2) ? 0 : withdrawAllowanceBefore + ); } - function test_withdrawOnBehalfOf_fuzz_allBalance(uint256 supplyAmount) public { + function test_withdrawOnBehalfOf_fuzz_allBalance( + uint256 supplyAmount, + uint256 approvalType + ) public { supplyAmount = bound(supplyAmount, 1, mintAmount_DAI); Utils.supply({ @@ -248,8 +296,14 @@ contract AllowancePositionManagerTest is SpokeBase { }); uint256 expectedSupplyShares = hub1.previewAddByAssets(daiAssetId, supplyAmount); - vm.prank(alice); - positionManager.approveWithdraw(bob, _daiReserveId(spoke), supplyAmount * 10); + approvalType = _fuzzyApproveWithdraw( + alice, + alicePk, + bob, + _daiReserveId(spoke), + supplyAmount * 10, + approvalType + ); uint256 prevUserBalance = tokenList.dai.balanceOf(alice); uint256 prevCallerBalance = tokenList.dai.balanceOf(bob); @@ -284,13 +338,14 @@ contract AllowancePositionManagerTest is SpokeBase { assertEq(tokenList.dai.allowance(address(positionManager), address(hub1)), 0); assertEq( positionManager.withdrawAllowance(alice, bob, _daiReserveId(spoke)), - prevAllowance - (supplyAmount * 2) + (approvalType < 2) ? prevAllowance - (supplyAmount * 2) : 0 ); } function test_withdrawOnBehalfOf_fuzz_allBalanceWithInterest( uint256 supplyAmount, - uint256 borrowAmount + uint256 borrowAmount, + uint256 approvalType ) public { supplyAmount = bound(supplyAmount, 2, mintAmount_DAI / 2); borrowAmount = bound(borrowAmount, 1, supplyAmount / 2); @@ -334,8 +389,14 @@ contract AllowancePositionManagerTest is SpokeBase { uint256 expectedWithdrawAmount = spoke.getUserSuppliedAssets(_daiReserveId(spoke), alice); - vm.prank(alice); - positionManager.approveWithdraw(bob, _daiReserveId(spoke), supplyAmount * 10); + _fuzzyApproveWithdraw( + alice, + alicePk, + bob, + _daiReserveId(spoke), + supplyAmount * 10, + approvalType + ); uint256 prevUserBalance = tokenList.dai.balanceOf(alice); uint256 prevCallerBalance = tokenList.dai.balanceOf(bob); @@ -370,8 +431,33 @@ contract AllowancePositionManagerTest is SpokeBase { assertEq(positionManager.withdrawAllowance(alice, bob, _daiReserveId(spoke)), 0); } - function test_withdrawOnBehalfOf_revertsWith_InsufficientWithdrawAllowance( - uint256 approvalAmount + // temporary withdraw allowance takes precedence over stored withdraw allowance, and does not cumulate + function test_withdrawOnBehalfOf_revertsWith_InsufficientWithdrawAllowance_TemporaryAllowanceTakesPrecedence() + public + { + uint256 storedAllowance = 300e18; + _fuzzyApproveWithdraw(alice, alicePk, bob, _daiReserveId(spoke), storedAllowance, 0); + + uint256 amount = 20e18; + uint256 temporaryAllowance = amount - 1; + _fuzzyApproveWithdraw(alice, alicePk, bob, _daiReserveId(spoke), temporaryAllowance, 2); + + vm.expectRevert( + abi.encodeWithSelector( + IAllowancePositionManager.InsufficientWithdrawAllowance.selector, + temporaryAllowance, + amount + ) + ); + vm.prank(bob); + positionManager.withdrawOnBehalfOf(_daiReserveId(spoke), amount, alice); + + assertEq(positionManager.withdrawAllowance(alice, bob, _daiReserveId(spoke)), storedAllowance); + } + + function test_withdrawOnBehalfOf_fuzz_revertsWith_InsufficientWithdrawAllowance( + uint256 approvalAmount, + uint256 approvalType ) public { uint256 amount = 100e18; approvalAmount = bound(approvalAmount, 1, amount - 1); @@ -384,8 +470,7 @@ contract AllowancePositionManagerTest is SpokeBase { onBehalfOf: alice }); - vm.prank(alice); - positionManager.approveWithdraw(bob, _daiReserveId(spoke), approvalAmount); + _fuzzyApproveWithdraw(alice, alicePk, bob, _daiReserveId(spoke), approvalAmount, approvalType); vm.expectRevert( abi.encodeWithSelector( @@ -432,7 +517,7 @@ contract AllowancePositionManagerTest is SpokeBase { ) public { vm.assume(spender != address(0)); reserveId = bound(reserveId, 0, spoke.getReserveCount() - 1); - amount = bound(amount, 1, mintAmount_DAI); + amount = bound(amount, 1, MAX_SUPPLY_AMOUNT); vm.expectEmit(address(positionManager)); emit IAllowancePositionManager.CreditDelegation(alice, spender, reserveId, amount); @@ -449,7 +534,7 @@ contract AllowancePositionManagerTest is SpokeBase { ) public { vm.assume(spender != address(0)); reserveId = bound(reserveId, 0, spoke.getReserveCount() - 1); - amount = bound(amount, 1, mintAmount_DAI); + amount = bound(amount, 1, MAX_SUPPLY_AMOUNT); EIP712Types.CreditDelegation memory p = _creditDelegationData( spender, @@ -503,7 +588,9 @@ contract AllowancePositionManagerTest is SpokeBase { positionManager.approveCreditDelegationWithSig(p, signature); } - function test_approveCreditDelegationWithSig_revertsWith_InvalidAccountNonce(bytes32) public { + function test_approveCreditDelegationWithSig_fuzz_revertsWith_InvalidAccountNonce( + bytes32 + ) public { EIP712Types.CreditDelegation memory p = _creditDelegationData( vm.randomAddress(), alice, @@ -522,9 +609,33 @@ contract AllowancePositionManagerTest is SpokeBase { positionManager.approveCreditDelegationWithSig(p, signature); } + function test_temporaryApproveCreditDelegation_fuzz( + address spender, + uint256 reserveId, + uint256 amount + ) public { + vm.assume(spender != address(0)); + reserveId = bound(reserveId, 0, spoke.getReserveCount() - 1); + amount = bound(amount, 1, MAX_SUPPLY_AMOUNT); + + vm.expectEmit(address(positionManager), 0); + vm.prank(alice); + positionManager.temporaryApproveCreditDelegation(spender, reserveId, amount); + + assertEq(positionManager.temporaryCreditDelegation(alice, spender, reserveId), amount); + } + + /// forge-config: default.isolate = true + function test_temporaryApproveCreditDelegation_TransientStorage() public { + // make sure transient storage is used for temporary credit delegations + vm.prank(alice); + positionManager.temporaryApproveCreditDelegation(bob, _daiReserveId(spoke), 100e18); + assertEq(positionManager.temporaryCreditDelegation(alice, bob, _daiReserveId(spoke)), 0); + } + function test_renounceCreditDelegation_fuzz(uint256 initialAllowance) public { uint256 reserveId = _randomReserveId(spoke); - initialAllowance = bound(initialAllowance, 1, mintAmount_DAI); + initialAllowance = bound(initialAllowance, 1, MAX_SUPPLY_AMOUNT); vm.prank(alice); positionManager.approveCreditDelegation(bob, reserveId, initialAllowance); @@ -538,10 +649,25 @@ contract AllowancePositionManagerTest is SpokeBase { } function test_borrowOnBehalfOf() public { - test_borrowOnBehalfOf_fuzz(5e18, 5e18); + test_borrowOnBehalfOf_fuzz(5e18, 5e18, 0); } - function test_borrowOnBehalfOf_fuzz(uint256 borrowAmount, uint256 creditDelegationAmount) public { + function test_borrowOnBehalfOf_TemporaryCreditDelegationTakesPrecedence() public { + uint256 storedAllowance = 300e18; + _fuzzyApproveCreditDelegation(alice, alicePk, bob, _daiReserveId(spoke), storedAllowance, 0); + test_borrowOnBehalfOf_fuzz(5e18, 5e18, 2); + // this check is also performed in test_borrowOnBehalfOf_fuzz, duplicating in case of future changes + assertEq( + positionManager.creditDelegationAllowance(alice, bob, _daiReserveId(spoke)), + storedAllowance + ); + } + + function test_borrowOnBehalfOf_fuzz( + uint256 borrowAmount, + uint256 creditDelegationAmount, + uint256 approvalType + ) public { uint256 aliceSupplyAmount = 5000e18; uint256 bobSupplyAmount = 1000e18; borrowAmount = bound(borrowAmount, 1, bobSupplyAmount); @@ -550,11 +676,18 @@ contract AllowancePositionManagerTest is SpokeBase { Utils.supplyCollateral(spoke, _daiReserveId(spoke), alice, aliceSupplyAmount, alice); Utils.supplyCollateral(spoke, _daiReserveId(spoke), bob, bobSupplyAmount, bob); - vm.prank(alice); - positionManager.approveCreditDelegation( - address(bob), + uint256 creditDelegationAllowanceBefore = positionManager.creditDelegationAllowance( + alice, + bob, + _daiReserveId(spoke) + ); + approvalType = _fuzzyApproveCreditDelegation( + alice, + alicePk, + bob, _daiReserveId(spoke), - creditDelegationAmount + creditDelegationAmount, + approvalType ); uint256 prevUserBalance = tokenList.dai.balanceOf(alice); @@ -591,23 +724,53 @@ contract AllowancePositionManagerTest is SpokeBase { assertEq(tokenList.dai.allowance(address(positionManager), address(hub1)), 0); assertEq( positionManager.creditDelegationAllowance(alice, bob, _daiReserveId(spoke)), - creditDelegationAmount - borrowAmount + (approvalType < 2) ? creditDelegationAmount - borrowAmount : creditDelegationAllowanceBefore ); } - function test_borrowOnBehalfOf_revertsWith_InsufficientCreditDelegation( - uint256 creditDelegationAmount + // temporary credit delegation takes precedence over stored credit delegation, and does not cumulate + function test_borrowOnBehalfOf_revertsWith_InsufficientCreditDelegation_TemporaryCreditDelegationTakesPrecedence() + public + { + uint256 storedAllowance = 300e18; + _fuzzyApproveCreditDelegation(alice, alicePk, bob, _daiReserveId(spoke), storedAllowance, 0); + + uint256 amount = 100e18; + uint256 temporaryAllowance = amount - 1; + _fuzzyApproveCreditDelegation(alice, alicePk, bob, _daiReserveId(spoke), temporaryAllowance, 2); + + vm.expectRevert( + abi.encodeWithSelector( + IAllowancePositionManager.InsufficientCreditDelegation.selector, + temporaryAllowance, + amount + ) + ); + vm.prank(bob); + positionManager.borrowOnBehalfOf(_daiReserveId(spoke), amount, alice); + + assertEq( + positionManager.creditDelegationAllowance(alice, bob, _daiReserveId(spoke)), + storedAllowance + ); + } + + function test_borrowOnBehalfOf_fuzz_revertsWith_InsufficientCreditDelegation( + uint256 creditDelegationAmount, + uint256 approvalType ) public { uint256 borrowAmount = 100e18; creditDelegationAmount = bound(creditDelegationAmount, 1, borrowAmount - 1); Utils.supplyCollateral(spoke, _daiReserveId(spoke), alice, borrowAmount, alice); Utils.supplyCollateral(spoke, _daiReserveId(spoke), bob, borrowAmount, bob); - vm.prank(alice); - positionManager.approveCreditDelegation( - address(bob), + _fuzzyApproveCreditDelegation( + alice, + alicePk, + bob, _daiReserveId(spoke), - creditDelegationAmount + creditDelegationAmount, + approvalType ); vm.expectRevert( @@ -638,6 +801,70 @@ contract AllowancePositionManagerTest is SpokeBase { positionManager.borrowOnBehalfOf(reserveId, 100e18, alice); } + function _fuzzyApproveWithdraw( + address onBehalfOf, + uint256 onBehalfOfPk, + address spender, + uint256 reserveId, + uint256 amount, + uint256 approvalType + ) internal returns (uint256) { + approvalType = bound(approvalType, 0, 2); + if (approvalType == 0) { + vm.prank(onBehalfOf); + positionManager.approveWithdraw(spender, reserveId, amount); + } else if (approvalType == 1) { + EIP712Types.WithdrawPermit memory p = _withdrawPermitData( + spender, + onBehalfOf, + type(uint256).max + ); + p.amount = amount; + p.reserveId = reserveId; + p.nonce = _burnRandomNoncesAtKey(positionManager, onBehalfOf); + bytes memory signature = _sign(onBehalfOfPk, _getTypedDataHash(positionManager, p)); + + vm.prank(vm.randomAddress()); + positionManager.approveWithdrawWithSig(p, signature); + } else { + vm.prank(onBehalfOf); + positionManager.temporaryApproveWithdraw(spender, reserveId, amount); + } + return approvalType; + } + + function _fuzzyApproveCreditDelegation( + address onBehalfOf, + uint256 onBehalfOfPk, + address spender, + uint256 reserveId, + uint256 amount, + uint256 approvalType + ) internal returns (uint256) { + approvalType = bound(approvalType, 0, 2); + if (approvalType == 0) { + vm.prank(onBehalfOf); + positionManager.approveCreditDelegation(spender, reserveId, amount); + } else if (approvalType == 1) { + EIP712Types.CreditDelegation memory p = _creditDelegationData( + spender, + onBehalfOf, + type(uint256).max + ); + p.amount = amount; + p.reserveId = reserveId; + p.nonce = _burnRandomNoncesAtKey(positionManager, onBehalfOf); + bytes memory signature = _sign(onBehalfOfPk, _getTypedDataHash(positionManager, p)); + + vm.prank(vm.randomAddress()); + positionManager.approveCreditDelegationWithSig(p, signature); + } else { + vm.prank(onBehalfOf); + positionManager.temporaryApproveCreditDelegation(spender, reserveId, amount); + } + return approvalType; + } + function _withdrawPermitData( address spender, address onBehalfOf, From 198c2b72feaac368ce0a7ed085fd3059f50d1172 Mon Sep 17 00:00:00 2001 From: Alexandru Niculae <43644109+avniculae@users.noreply.github.com> Date: Mon, 15 Dec 2025 15:17:02 +0200 Subject: [PATCH 2/8] feat: add transient slot collision test --- .../unit/position-managers/AllowancePositionManager.t.sol | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/position-managers/AllowancePositionManager.t.sol b/tests/unit/position-managers/AllowancePositionManager.t.sol index 3048442b0..f51ef23b0 100644 --- a/tests/unit/position-managers/AllowancePositionManager.t.sol +++ b/tests/unit/position-managers/AllowancePositionManager.t.sol @@ -815,6 +815,13 @@ contract AllowancePositionManagerTest is SpokeBase { positionManager.borrowOnBehalfOf(reserveId, 100e18, alice); } + function test_temporaryAllowancesInParallel() public { + _fuzzyApproveWithdraw(alice, alicePk, bob, _daiReserveId(spoke), 1e18, 2); + _fuzzyCreditDelegation(alice, alicePk, bob, _daiReserveId(spoke), 2e18, 2); + assertEq(positionManager.temporaryWithdrawAllowance(alice, bob, _daiReserveId(spoke)), 1e18); + assertEq(positionManager.temporaryCreditDelegation(alice, bob, _daiReserveId(spoke)), 2e18); + } + function _fuzzyApproveWithdraw( address onBehalfOf, uint256 onBehalfOfPk, From f7a36b288e18595ac50c0120a905e898a0fd6dbd Mon Sep 17 00:00:00 2001 From: Alexandru Niculae <43644109+avniculae@users.noreply.github.com> Date: Mon, 15 Dec 2025 15:22:45 +0200 Subject: [PATCH 3/8] test: remove unused code --- tests/unit/position-managers/AllowancePositionManager.t.sol | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/unit/position-managers/AllowancePositionManager.t.sol b/tests/unit/position-managers/AllowancePositionManager.t.sol index f51ef23b0..3670823c8 100644 --- a/tests/unit/position-managers/AllowancePositionManager.t.sol +++ b/tests/unit/position-managers/AllowancePositionManager.t.sol @@ -11,11 +11,6 @@ contract AllowancePositionManagerTest is SpokeBase { TestReturnValues public returnValues; uint256 public alicePk; - bytes32 private constant _TEMPORARY_WITHDRAW_ALLOWANCES_SLOT = - 0x1c6a61279a13a86a789311ddf30aee38e2f4a9f6c4aad1ff4a2e75a4018e68c3; - bytes32 private constant _TEMPORARY_CREDIT_DELEGATIONS_SLOT = - 0xcd470af8670f5baa744a0341af8a2e3f5d7ca086178908432a5cfaf39cb9299d; - function setUp() public virtual override { deployFixtures(); initEnvironment(); From 26743fa948970c2b73e7612e83d501d850ffcb93 Mon Sep 17 00:00:00 2001 From: Alexandru Niculae <43644109+avniculae@users.noreply.github.com> Date: Mon, 15 Dec 2025 19:51:03 +0200 Subject: [PATCH 4/8] feat: gas snapshots --- snapshots/PositionManager.Operations.json | 6 +- .../gas/PositionManagers.Operations.gas.t.sol | 61 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/snapshots/PositionManager.Operations.json b/snapshots/PositionManager.Operations.json index 6030efdd0..18b88cb71 100644 --- a/snapshots/PositionManager.Operations.json +++ b/snapshots/PositionManager.Operations.json @@ -1,13 +1,17 @@ { "AllowancePositionManager: approveWithdraw": "46880", "AllowancePositionManager: approveWithdrawWithSig": "63091", - "AllowancePositionManager: borrowOnBehalfOf": "309405", + "AllowancePositionManager: borrowOnBehalfOf": "247372", "AllowancePositionManager: creditDelegation": "46858", "AllowancePositionManager: creditDelegationWithSig": "63046", "AllowancePositionManager: renounceCreditDelegation": "24900", "AllowancePositionManager: renounceWithdrawAllowance": "24922", + "AllowancePositionManager: temporaryApproveWithdraw": "22702", + "AllowancePositionManager: temporaryCreditDelegation": "22681", "AllowancePositionManager: withdrawOnBehalfOf: full": "123889", + "AllowancePositionManager: withdrawOnBehalfOf: full (with temporary allowance)": "56228", "AllowancePositionManager: withdrawOnBehalfOf: partial": "134661", + "AllowancePositionManager: withdrawOnBehalfOf: partial (with temporary allowance)": "67028", "PositionConfigPositionManager: renounceGlobalPermission": "24445", "PositionConfigPositionManager: renounceUserDynamicConfigPermission": "24490", "PositionConfigPositionManager: renounceUserRiskPremiumPermission": "24489", diff --git a/tests/gas/PositionManagers.Operations.gas.t.sol b/tests/gas/PositionManagers.Operations.gas.t.sol index dc502928b..d346e1b16 100644 --- a/tests/gas/PositionManagers.Operations.gas.t.sol +++ b/tests/gas/PositionManagers.Operations.gas.t.sol @@ -135,6 +135,34 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase { vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: withdrawOnBehalfOf: full'); } + /// forge-config: default.isolate = false + function test_withdrawOnBehalfOf_WithTemporaryWithdrawAllowance() public { + uint256 amount = 100e18; + + vm.prank(alice); + positionManager.temporaryApproveWithdraw(bob, _daiReserveId(spoke1), UINT256_MAX); + + Utils.supply(spoke1, _daiReserveId(spoke1), alice, mintAmount_DAI, alice); + Utils.withdraw(spoke1, _daiReserveId(spoke1), alice, amount, alice); + + vm.prank(bob); + positionManager.withdrawOnBehalfOf(_daiReserveId(spoke1), amount, alice); + vm.snapshotGasLastCall( + NAMESPACE, + 'AllowancePositionManager: withdrawOnBehalfOf: partial (with temporary allowance)' + ); + + vm.prank(alice); + positionManager.temporaryApproveWithdraw(bob, _daiReserveId(spoke1), UINT256_MAX); + + vm.prank(bob); + positionManager.withdrawOnBehalfOf(_daiReserveId(spoke1), UINT256_MAX, alice); + vm.snapshotGasLastCall( + NAMESPACE, + 'AllowancePositionManager: withdrawOnBehalfOf: full (with temporary allowance)' + ); + } + function test_borrowOnBehalfOf() public { uint256 aliceSupplyAmount = 5000e18; uint256 bobSupplyAmount = 1000e18; @@ -151,6 +179,23 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase { vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: borrowOnBehalfOf'); } + /// forge-config: default.isolate = false + function test_borrowOnBehalfOf_WithTemporaryCreditDelegation() public { + uint256 aliceSupplyAmount = 5000e18; + uint256 bobSupplyAmount = 1000e18; + uint256 borrowAmount = 750e18; + + vm.prank(alice); + positionManager.temporaryCreditDelegation(bob, _daiReserveId(spoke1), borrowAmount); + + Utils.supplyCollateral(spoke1, _daiReserveId(spoke1), alice, aliceSupplyAmount, alice); + Utils.supplyCollateral(spoke1, _daiReserveId(spoke1), bob, bobSupplyAmount, bob); + + vm.prank(bob); + positionManager.borrowOnBehalfOf(_daiReserveId(spoke1), borrowAmount, alice); + vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: borrowOnBehalfOf'); + } + function test_approveWithdraw() public { uint256 amount = 100e18; @@ -182,6 +227,14 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase { vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: approveWithdrawWithSig'); } + function test_temporaryApproveWithdraw() public { + uint256 amount = 100e18; + + vm.prank(alice); + positionManager.temporaryApproveWithdraw(bob, _daiReserveId(spoke1), amount); + vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: temporaryApproveWithdraw'); + } + function test_renounceWithdrawAllowance() public { uint256 amount = 100e18; @@ -224,6 +277,14 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase { vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: creditDelegationWithSig'); } + function test_temporaryCreditDelegation() public { + uint256 amount = 100e18; + + vm.prank(alice); + positionManager.temporaryCreditDelegation(bob, _daiReserveId(spoke1), amount); + vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: temporaryCreditDelegation'); + } + function test_renounceCreditDelegation() public { uint256 amount = 100e18; From 53340e32a2e65fb52255c6a1a6f85c6fcdaf3e11 Mon Sep 17 00:00:00 2001 From: Alexandru Niculae <43644109+avniculae@users.noreply.github.com> Date: Thu, 18 Dec 2025 13:31:03 +0200 Subject: [PATCH 5/8] fix: comments --- .../AllowancePositionManager.Operations.json | 13 ++++--- snapshots/PositionManager.Operations.json | 29 --------------- snapshots/SignatureGateway.Operations.json | 2 +- .../Spoke.Operations.ZeroRiskPremium.json | 2 +- snapshots/Spoke.Operations.json | 2 +- .../AllowancePositionManager.sol | 30 ++++++++++------ .../interfaces/IAllowancePositionManager.sol | 8 +++++ .../gas/PositionManagers.Operations.gas.t.sol | 16 +++------ .../AllowancePositionManager.t.sol | 35 +++++++++++-------- 9 files changed, 62 insertions(+), 75 deletions(-) delete mode 100644 snapshots/PositionManager.Operations.json diff --git a/snapshots/AllowancePositionManager.Operations.json b/snapshots/AllowancePositionManager.Operations.json index 580857cdd..e6df435a4 100644 --- a/snapshots/AllowancePositionManager.Operations.json +++ b/snapshots/AllowancePositionManager.Operations.json @@ -1,16 +1,15 @@ { - "AllowancePositionManager: borrowOnBehalfOf": "247328", - "AllowancePositionManager: temporaryApproveWithdraw": "22680", - "AllowancePositionManager: temporaryDelegateCredit": "22768", - "AllowancePositionManager: withdrawOnBehalfOf: full (with temporary allowance)": "56184", - "AllowancePositionManager: withdrawOnBehalfOf: partial (with temporary allowance)": "66984", "approveWithdraw": "46795", "approveWithdrawWithSig": "63006", - "borrowOnBehalfOf": "307050", + "borrowOnBehalfOf": "247328", "delegateCredit": "46784", "delegateCreditWithSig": "63081", "renounceCreditDelegation": "24937", "renounceWithdrawAllowance": "24837", + "temporaryApproveWithdraw": "22680", + "temporaryDelegateCredit": "22768", "withdrawOnBehalfOf: full": "122006", - "withdrawOnBehalfOf: partial": "132307" + "withdrawOnBehalfOf: full (with temporary allowance)": "56184", + "withdrawOnBehalfOf: partial": "132307", + "withdrawOnBehalfOf: partial (with temporary allowance)": "66984" } \ No newline at end of file diff --git a/snapshots/PositionManager.Operations.json b/snapshots/PositionManager.Operations.json deleted file mode 100644 index 18b88cb71..000000000 --- a/snapshots/PositionManager.Operations.json +++ /dev/null @@ -1,29 +0,0 @@ -{ - "AllowancePositionManager: approveWithdraw": "46880", - "AllowancePositionManager: approveWithdrawWithSig": "63091", - "AllowancePositionManager: borrowOnBehalfOf": "247372", - "AllowancePositionManager: creditDelegation": "46858", - "AllowancePositionManager: creditDelegationWithSig": "63046", - "AllowancePositionManager: renounceCreditDelegation": "24900", - "AllowancePositionManager: renounceWithdrawAllowance": "24922", - "AllowancePositionManager: temporaryApproveWithdraw": "22702", - "AllowancePositionManager: temporaryCreditDelegation": "22681", - "AllowancePositionManager: withdrawOnBehalfOf: full": "123889", - "AllowancePositionManager: withdrawOnBehalfOf: full (with temporary allowance)": "56228", - "AllowancePositionManager: withdrawOnBehalfOf: partial": "134661", - "AllowancePositionManager: withdrawOnBehalfOf: partial (with temporary allowance)": "67028", - "PositionConfigPositionManager: renounceGlobalPermission": "24445", - "PositionConfigPositionManager: renounceUserDynamicConfigPermission": "24490", - "PositionConfigPositionManager: renounceUserRiskPremiumPermission": "24489", - "PositionConfigPositionManager: renounceUsingAsCollateralPermission": "24423", - "PositionConfigPositionManager: setGlobalPermission": "46541", - "PositionConfigPositionManager: setUserDynamicConfigPermission": "46585", - "PositionConfigPositionManager: setUserRiskPremiumPermission": "46564", - "PositionConfigPositionManager: setUsingAsCollateralOnBehalfOf": "69074", - "PositionConfigPositionManager: setUsingAsCollateralPermission": "46540", - "PositionConfigPositionManager: updateUserDynamicConfigOnBehalfOf": "46787", - "PositionConfigPositionManager: updateUserRiskPremiumOnBehalfOf": "127859", - "SupplyRepayPositionManager: repayOnBehalfOf": "167131", - "SupplyRepayPositionManager: supplyOnBehalfOf": "135820", - "common: setSelfAsUserPositionManagerWithSig": "71977" -} \ No newline at end of file diff --git a/snapshots/SignatureGateway.Operations.json b/snapshots/SignatureGateway.Operations.json index 2cbf44a58..b984c36ee 100644 --- a/snapshots/SignatureGateway.Operations.json +++ b/snapshots/SignatureGateway.Operations.json @@ -1,5 +1,5 @@ { - "borrowWithSig": "215605", + "borrowWithSig": "215593", "repayWithSig": "188872", "setSelfAsUserPositionManagerWithSig": "74858", "setUsingAsCollateralWithSig": "85053", diff --git a/snapshots/Spoke.Operations.ZeroRiskPremium.json b/snapshots/Spoke.Operations.ZeroRiskPremium.json index ed2faa23d..ba5d6d7d3 100644 --- a/snapshots/Spoke.Operations.ZeroRiskPremium.json +++ b/snapshots/Spoke.Operations.ZeroRiskPremium.json @@ -11,7 +11,7 @@ "repay: full": "126094", "repay: partial": "130983", "setUserPositionManagerWithSig: disable": "44846", - "setUserPositionManagerWithSig: enable": "68875", + "setUserPositionManagerWithSig: enable": "68863", "supply + enable collateral (multicall)": "140624", "supply: 0 borrows, collateral disabled": "123679", "supply: 0 borrows, collateral enabled": "106601", diff --git a/snapshots/Spoke.Operations.json b/snapshots/Spoke.Operations.json index deed9c95d..07354ba44 100644 --- a/snapshots/Spoke.Operations.json +++ b/snapshots/Spoke.Operations.json @@ -11,7 +11,7 @@ "repay: full": "120256", "repay: partial": "139545", "setUserPositionManagerWithSig: disable": "44846", - "setUserPositionManagerWithSig: enable": "68875", + "setUserPositionManagerWithSig: enable": "68863", "supply + enable collateral (multicall)": "140624", "supply: 0 borrows, collateral disabled": "123679", "supply: 0 borrows, collateral enabled": "106601", diff --git a/src/position-manager/AllowancePositionManager.sol b/src/position-manager/AllowancePositionManager.sol index e525e86cb..ae263af08 100644 --- a/src/position-manager/AllowancePositionManager.sol +++ b/src/position-manager/AllowancePositionManager.sol @@ -84,7 +84,8 @@ contract AllowancePositionManager is /// @inheritdoc IAllowancePositionManager function temporaryApproveWithdraw(address spender, uint256 reserveId, uint256 amount) external { - _temporaryWithdrawAllowancesSlot(msg.sender, spender, reserveId).tstore(amount); + _temporaryWithdrawAllowancesSlot({owner: msg.sender, spender: spender, reserveId: reserveId}) + .tstore(amount); } /// @inheritdoc IAllowancePositionManager @@ -120,7 +121,8 @@ contract AllowancePositionManager is /// @inheritdoc IAllowancePositionManager function temporaryDelegateCredit(address spender, uint256 reserveId, uint256 amount) external { - _temporaryDelegateCreditsSlot(msg.sender, spender, reserveId).tstore(amount); + _temporaryDelegateCreditsSlot({owner: msg.sender, spender: spender, reserveId: reserveId}) + .tstore(amount); } /// @inheritdoc IAllowancePositionManager @@ -237,16 +239,18 @@ contract AllowancePositionManager is uint256 reserveId, uint256 amount ) internal { - uint256 temporaryAllowance = _temporaryWithdrawAllowancesSlot(owner, spender, reserveId) - .tload(); + uint256 temporaryAllowance = _temporaryWithdrawAllowancesSlot({ + owner: owner, + spender: spender, + reserveId: reserveId + }).tload(); if (temporaryAllowance > 0) { require( temporaryAllowance >= amount, - InsufficientWithdrawAllowance(temporaryAllowance, amount) - ); - _temporaryWithdrawAllowancesSlot(owner, spender, reserveId).tstore( - temporaryAllowance.uncheckedSub(amount) + InsufficientTemporaryWithdrawAllowance(temporaryAllowance, amount) ); + _temporaryWithdrawAllowancesSlot({owner: owner, spender: spender, reserveId: reserveId}) + .tstore(temporaryAllowance.uncheckedSub(amount)); } else { uint256 allowance = _withdrawAllowances[owner][spender][reserveId]; require(allowance >= amount, InsufficientWithdrawAllowance(allowance, amount)); @@ -261,13 +265,17 @@ contract AllowancePositionManager is uint256 reserveId, uint256 amount ) internal { - uint256 temporaryAllowance = _temporaryDelegateCreditsSlot(owner, spender, reserveId).tload(); + uint256 temporaryAllowance = _temporaryDelegateCreditsSlot({ + owner: owner, + spender: spender, + reserveId: reserveId + }).tload(); if (temporaryAllowance > 0) { require( temporaryAllowance >= amount, - InsufficientCreditDelegation(temporaryAllowance, amount) + InsufficientTemporaryCreditDelegation(temporaryAllowance, amount) ); - _temporaryDelegateCreditsSlot(owner, spender, reserveId).tstore( + _temporaryDelegateCreditsSlot({owner: owner, spender: spender, reserveId: reserveId}).tstore( temporaryAllowance.uncheckedSub(amount) ); } else { diff --git a/src/position-manager/interfaces/IAllowancePositionManager.sol b/src/position-manager/interfaces/IAllowancePositionManager.sol index 9637d062b..f662104be 100644 --- a/src/position-manager/interfaces/IAllowancePositionManager.sol +++ b/src/position-manager/interfaces/IAllowancePositionManager.sol @@ -11,8 +11,12 @@ import {IPositionManagerBase} from 'src/position-manager/interfaces/IPositionMan interface IAllowancePositionManager is IPositionManagerBase { /// @notice Thrown when the withdraw allowance is insufficient. error InsufficientWithdrawAllowance(uint256 allowance, uint256 required); + /// @notice Thrown when the temporary withdraw allowance is insufficient. + error InsufficientTemporaryWithdrawAllowance(uint256 allowance, uint256 required); /// @notice Thrown when the credit delegation allowance is insufficient. error InsufficientCreditDelegation(uint256 allowance, uint256 required); + /// @notice Thrown when the temporary credit delegation allowance is insufficient. + error InsufficientTemporaryCreditDelegation(uint256 allowance, uint256 required); /// @notice Emitted when `owner` approves `spender` to withdraw `amount` for `reserveId` on their behalf. /// @param owner The address of the owner. @@ -53,6 +57,7 @@ interface IAllowancePositionManager is IPositionManagerBase { ) external; /// @notice Temporarily approves a spender to withdraw assets from the specified reserve on the spoke. + /// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate. /// @dev The allowance is discarded after the transaction. /// @param spender The address of the spender to receive the allowance. /// @param reserveId The identifier of the reserve. @@ -74,6 +79,7 @@ interface IAllowancePositionManager is IPositionManagerBase { ) external; /// @notice Temporarily approves a credit delegation allowance for a spender. + /// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate. /// @dev The allowance is discarded after the transaction. /// @param spender The address of the spender to receive the allowance. /// @param reserveId The identifier of the reserve. @@ -92,6 +98,7 @@ interface IAllowancePositionManager is IPositionManagerBase { /// @notice Executes a withdraw on behalf of a user. /// @dev The caller must have sufficient withdraw allowance from `onBehalfOf`. + /// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate. /// @dev The caller receives the withdrawn assets. /// @param reserveId The identifier of the reserve. /// @param amount The amount to withdraw. @@ -106,6 +113,7 @@ interface IAllowancePositionManager is IPositionManagerBase { /// @notice Executes a borrow on behalf of a user. /// @dev The caller must have sufficient credit delegation allowance from `onBehalfOf`. + /// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate. /// @dev The caller receives the borrowed assets. /// @param reserveId The identifier of the reserve. /// @param amount The amount to borrow. diff --git a/tests/gas/PositionManagers.Operations.gas.t.sol b/tests/gas/PositionManagers.Operations.gas.t.sol index afba860d9..b2e37cb3d 100644 --- a/tests/gas/PositionManagers.Operations.gas.t.sol +++ b/tests/gas/PositionManagers.Operations.gas.t.sol @@ -147,20 +147,14 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase { vm.prank(bob); positionManager.withdrawOnBehalfOf(_daiReserveId(spoke1), amount, alice); - vm.snapshotGasLastCall( - NAMESPACE, - 'AllowancePositionManager: withdrawOnBehalfOf: partial (with temporary allowance)' - ); + vm.snapshotGasLastCall(NAMESPACE, 'withdrawOnBehalfOf: partial (with temporary allowance)'); vm.prank(alice); positionManager.temporaryApproveWithdraw(bob, _daiReserveId(spoke1), UINT256_MAX); vm.prank(bob); positionManager.withdrawOnBehalfOf(_daiReserveId(spoke1), UINT256_MAX, alice); - vm.snapshotGasLastCall( - NAMESPACE, - 'AllowancePositionManager: withdrawOnBehalfOf: full (with temporary allowance)' - ); + vm.snapshotGasLastCall(NAMESPACE, 'withdrawOnBehalfOf: full (with temporary allowance)'); } function test_borrowOnBehalfOf() public { @@ -193,7 +187,7 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase { vm.prank(bob); positionManager.borrowOnBehalfOf(_daiReserveId(spoke1), borrowAmount, alice); - vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: borrowOnBehalfOf'); + vm.snapshotGasLastCall(NAMESPACE, 'borrowOnBehalfOf'); } function test_approveWithdraw() public { @@ -232,7 +226,7 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase { vm.prank(alice); positionManager.temporaryApproveWithdraw(bob, _daiReserveId(spoke1), amount); - vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: temporaryApproveWithdraw'); + vm.snapshotGasLastCall(NAMESPACE, 'temporaryApproveWithdraw'); } function test_renounceWithdrawAllowance() public { @@ -282,7 +276,7 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase { vm.prank(alice); positionManager.temporaryDelegateCredit(bob, _daiReserveId(spoke1), amount); - vm.snapshotGasLastCall(NAMESPACE, 'AllowancePositionManager: temporaryDelegateCredit'); + vm.snapshotGasLastCall(NAMESPACE, 'temporaryDelegateCredit'); } function test_renounceCreditDelegation() public { diff --git a/tests/unit/position-managers/AllowancePositionManager.t.sol b/tests/unit/position-managers/AllowancePositionManager.t.sol index 1c1912f34..b2853e1cd 100644 --- a/tests/unit/position-managers/AllowancePositionManager.t.sol +++ b/tests/unit/position-managers/AllowancePositionManager.t.sol @@ -440,9 +440,7 @@ contract AllowancePositionManagerTest is SpokeBase { } // temporary withdraw allowance takes precedence over stored withdraw allowance, and does not cumulate - function test_withdrawOnBehalfOf_revertsWith_InsufficientWithdrawAllowance_TemporaryAllowanceTakesPrecedence() - public - { + function test_withdrawOnBehalfOf_revertsWith_InsufficientTemporaryWithdrawAllowance() public { uint256 storedAllowance = 300e18; _fuzzyApproveWithdraw(alice, alicePk, bob, _daiReserveId(spoke), storedAllowance, 0); @@ -452,7 +450,7 @@ contract AllowancePositionManagerTest is SpokeBase { vm.expectRevert( abi.encodeWithSelector( - IAllowancePositionManager.InsufficientWithdrawAllowance.selector, + IAllowancePositionManager.InsufficientTemporaryWithdrawAllowance.selector, temporaryAllowance, amount ) @@ -463,7 +461,7 @@ contract AllowancePositionManagerTest is SpokeBase { assertEq(positionManager.withdrawAllowance(alice, bob, _daiReserveId(spoke)), storedAllowance); } - function test_withdrawOnBehalfOf_fuzz_revertsWith_InsufficientWithdrawAllowance( + function test_withdrawOnBehalfOf_fuzz_revertsWith_InsufficientAllowance( uint256 approvalAmount, uint256 approvalType ) public { @@ -478,11 +476,20 @@ contract AllowancePositionManagerTest is SpokeBase { onBehalfOf: alice }); - _fuzzyApproveWithdraw(alice, alicePk, bob, _daiReserveId(spoke), approvalAmount, approvalType); + approvalType = _fuzzyApproveWithdraw( + alice, + alicePk, + bob, + _daiReserveId(spoke), + approvalAmount, + approvalType + ); vm.expectRevert( abi.encodeWithSelector( - IAllowancePositionManager.InsufficientWithdrawAllowance.selector, + (approvalType == 2) + ? IAllowancePositionManager.InsufficientTemporaryWithdrawAllowance.selector + : IAllowancePositionManager.InsufficientWithdrawAllowance.selector, approvalAmount, amount ) @@ -734,9 +741,7 @@ contract AllowancePositionManagerTest is SpokeBase { } // temporary credit delegation takes precedence over stored credit delegation, and does not cumulate - function test_borrowOnBehalfOf_revertsWith_InsufficientCreditDelegation_temporaryDelegateCreditTakesPrecedence() - public - { + function test_borrowOnBehalfOf_revertsWith_InsufficientTemporaryCreditDelegation() public { uint256 storedAllowance = 300e18; _fuzzyDelegateCredit(alice, alicePk, bob, _daiReserveId(spoke), storedAllowance, 0); @@ -746,7 +751,7 @@ contract AllowancePositionManagerTest is SpokeBase { vm.expectRevert( abi.encodeWithSelector( - IAllowancePositionManager.InsufficientCreditDelegation.selector, + IAllowancePositionManager.InsufficientTemporaryCreditDelegation.selector, temporaryAllowance, amount ) @@ -757,7 +762,7 @@ contract AllowancePositionManagerTest is SpokeBase { assertEq(positionManager.creditDelegation(alice, bob, _daiReserveId(spoke)), storedAllowance); } - function test_borrowOnBehalfOf_fuzz_revertsWith_InsufficientCreditDelegation( + function test_borrowOnBehalfOf_fuzz_revertsWith_InsufficientAllowance( uint256 creditDelegationAmount, uint256 approvalType ) public { @@ -766,7 +771,7 @@ contract AllowancePositionManagerTest is SpokeBase { Utils.supplyCollateral(spoke, _daiReserveId(spoke), alice, borrowAmount, alice); Utils.supplyCollateral(spoke, _daiReserveId(spoke), bob, borrowAmount, bob); - _fuzzyDelegateCredit( + approvalType = _fuzzyDelegateCredit( alice, alicePk, bob, @@ -777,7 +782,9 @@ contract AllowancePositionManagerTest is SpokeBase { vm.expectRevert( abi.encodeWithSelector( - IAllowancePositionManager.InsufficientCreditDelegation.selector, + (approvalType == 2) + ? IAllowancePositionManager.InsufficientTemporaryCreditDelegation.selector + : IAllowancePositionManager.InsufficientCreditDelegation.selector, creditDelegationAmount, borrowAmount ) From 6e766f69661887851d8ef9dd60f1c005c84182a4 Mon Sep 17 00:00:00 2001 From: Alexandru Niculae <43644109+avniculae@users.noreply.github.com> Date: Thu, 18 Dec 2025 13:38:44 +0200 Subject: [PATCH 6/8] fix: gas snapshot --- snapshots/AllowancePositionManager.Operations.json | 3 ++- snapshots/SignatureGateway.Operations.json | 2 +- snapshots/Spoke.Operations.ZeroRiskPremium.json | 2 +- snapshots/Spoke.Operations.json | 2 +- tests/gas/PositionManagers.Operations.gas.t.sol | 4 ++-- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/snapshots/AllowancePositionManager.Operations.json b/snapshots/AllowancePositionManager.Operations.json index e6df435a4..96c1f4b92 100644 --- a/snapshots/AllowancePositionManager.Operations.json +++ b/snapshots/AllowancePositionManager.Operations.json @@ -1,7 +1,8 @@ { "approveWithdraw": "46795", "approveWithdrawWithSig": "63006", - "borrowOnBehalfOf": "247328", + "borrowOnBehalfOf": "307050", + "borrowOnBehalfOf (with temporary allowance)": "247328", "delegateCredit": "46784", "delegateCreditWithSig": "63081", "renounceCreditDelegation": "24937", diff --git a/snapshots/SignatureGateway.Operations.json b/snapshots/SignatureGateway.Operations.json index b984c36ee..2cbf44a58 100644 --- a/snapshots/SignatureGateway.Operations.json +++ b/snapshots/SignatureGateway.Operations.json @@ -1,5 +1,5 @@ { - "borrowWithSig": "215593", + "borrowWithSig": "215605", "repayWithSig": "188872", "setSelfAsUserPositionManagerWithSig": "74858", "setUsingAsCollateralWithSig": "85053", diff --git a/snapshots/Spoke.Operations.ZeroRiskPremium.json b/snapshots/Spoke.Operations.ZeroRiskPremium.json index ba5d6d7d3..ed2faa23d 100644 --- a/snapshots/Spoke.Operations.ZeroRiskPremium.json +++ b/snapshots/Spoke.Operations.ZeroRiskPremium.json @@ -11,7 +11,7 @@ "repay: full": "126094", "repay: partial": "130983", "setUserPositionManagerWithSig: disable": "44846", - "setUserPositionManagerWithSig: enable": "68863", + "setUserPositionManagerWithSig: enable": "68875", "supply + enable collateral (multicall)": "140624", "supply: 0 borrows, collateral disabled": "123679", "supply: 0 borrows, collateral enabled": "106601", diff --git a/snapshots/Spoke.Operations.json b/snapshots/Spoke.Operations.json index 07354ba44..deed9c95d 100644 --- a/snapshots/Spoke.Operations.json +++ b/snapshots/Spoke.Operations.json @@ -11,7 +11,7 @@ "repay: full": "120256", "repay: partial": "139545", "setUserPositionManagerWithSig: disable": "44846", - "setUserPositionManagerWithSig: enable": "68863", + "setUserPositionManagerWithSig: enable": "68875", "supply + enable collateral (multicall)": "140624", "supply: 0 borrows, collateral disabled": "123679", "supply: 0 borrows, collateral enabled": "106601", diff --git a/tests/gas/PositionManagers.Operations.gas.t.sol b/tests/gas/PositionManagers.Operations.gas.t.sol index b2e37cb3d..65300adeb 100644 --- a/tests/gas/PositionManagers.Operations.gas.t.sol +++ b/tests/gas/PositionManagers.Operations.gas.t.sol @@ -174,7 +174,7 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase { } /// forge-config: default.isolate = false - function test_borrowOnBehalfOf_WithtemporaryDelegateCredit() public { + function test_borrowOnBehalfOf_WithTemporaryDelegateCredit() public { uint256 aliceSupplyAmount = 5000e18; uint256 bobSupplyAmount = 1000e18; uint256 borrowAmount = 750e18; @@ -187,7 +187,7 @@ contract AllowancePositionManager_Gas_Tests is SpokeBase { vm.prank(bob); positionManager.borrowOnBehalfOf(_daiReserveId(spoke1), borrowAmount, alice); - vm.snapshotGasLastCall(NAMESPACE, 'borrowOnBehalfOf'); + vm.snapshotGasLastCall(NAMESPACE, 'borrowOnBehalfOf (with temporary allowance)'); } function test_approveWithdraw() public { From 01225ac05612e3fc04152cf9937d5db0055fc5f5 Mon Sep 17 00:00:00 2001 From: Alexandru Niculae <43644109+avniculae@users.noreply.github.com> Date: Thu, 18 Dec 2025 14:37:52 +0200 Subject: [PATCH 7/8] fix: transient slot derivation --- .../AllowancePositionManager.sol | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/position-manager/AllowancePositionManager.sol b/src/position-manager/AllowancePositionManager.sol index ae263af08..f086ebfae 100644 --- a/src/position-manager/AllowancePositionManager.sol +++ b/src/position-manager/AllowancePositionManager.sol @@ -29,24 +29,24 @@ contract AllowancePositionManager is using SlotDerivation for bytes32; using TransientSlot for *; + /// @notice Slot for the temporary withdraw allowances. + /// @dev keccak256(abi.encode(uint256(keccak256("aave.transient.WITHDRAW_ALLOWANCES")) - 1)) & ~bytes32(uint256(0xff)) + bytes32 private constant _TEMPORARY_WITHDRAW_ALLOWANCES_SLOT = + 0x4b5553e643854b1bacc0d454fec49da235a0faac2caff4f059541ccf9f154700; + + /// @notice Slot for the temporary credit delegations. + /// @dev keccak256(abi.encode(uint256(keccak256("aave.transient.CREDIT_DELEGATIONS")) - 1)) & ~bytes32(uint256(0xff)) + bytes32 private constant _TEMPORARY_CREDIT_DELEGATIONS_SLOT = + 0x5aa827cbd079fec1557555542f5232f82e413903ea6ea8e935f719e23b7c4a00; + /// @notice Mapping of withdraw allowances. mapping(address owner => mapping(address spender => mapping(uint256 reserveId => uint256 amount))) private _withdrawAllowances; - /// @notice Slot for the temporary withdraw allowances. - /// @dev keccak256('temporary.withdrawAllowances') - bytes32 private constant _TEMPORARY_WITHDRAW_ALLOWANCES_SLOT = - 0x1c6a61279a13a86a789311ddf30aee38e2f4a9f6c4aad1ff4a2e75a4018e68c3; - /// @notice Mapping of credit delegations. mapping(address owner => mapping(address spender => mapping(uint256 reserveId => uint256 amount))) private _creditDelegations; - /// @notice Slot for the temporary credit delegations. - /// @dev keccak256('temporary.creditDelegations') - bytes32 private constant _TEMPORARY_CREDIT_DELEGATIONS_SLOT = - 0xcd470af8670f5baa744a0341af8a2e3f5d7ca086178908432a5cfaf39cb9299d; - /// @dev Constructor. /// @param spoke_ The address of the spoke contract. constructor(address spoke_) PositionManagerBase(spoke_) {} From a96826ce1cdfa562ada0ad399c1e5babc73ebfec Mon Sep 17 00:00:00 2001 From: Alexandru Niculae <43644109+avniculae@users.noreply.github.com> Date: Fri, 19 Dec 2025 20:09:05 +0200 Subject: [PATCH 8/8] fix: bad merge --- src/position-manager/AllowancePositionManager.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/position-manager/AllowancePositionManager.sol b/src/position-manager/AllowancePositionManager.sol index 7b2b2a555..da4242c39 100644 --- a/src/position-manager/AllowancePositionManager.sol +++ b/src/position-manager/AllowancePositionManager.sol @@ -303,9 +303,9 @@ contract AllowancePositionManager is /// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate. function _spendWithdrawAllowance( address spoke, + uint256 reserveId, address owner, address spender, - uint256 reserveId, uint256 amount ) internal { uint256 temporaryAllowance = _temporaryWithdrawAllowancesSlot({ @@ -339,9 +339,9 @@ contract AllowancePositionManager is /// @dev Temporary allowance takes precedence over stored allowance, and does not cumulate. function _spendCreditDelegation( address spoke, + uint256 reserveId, address owner, address spender, - uint256 reserveId, uint256 amount ) internal { uint256 temporaryAllowance = _temporaryDelegateCreditsSlot({