Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions snapshots/PositionManager.Operations.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
{
"AllowancePositionManager: approveWithdraw": "46858",
"AllowancePositionManager: approveWithdrawWithSig": "63069",
"AllowancePositionManager: borrowOnBehalfOf": "308815",
"AllowancePositionManager: creditDelegation": "46836",
"AllowancePositionManager: approveWithdraw": "46880",
"AllowancePositionManager: approveWithdrawWithSig": "63091",
"AllowancePositionManager: borrowOnBehalfOf": "247372",
"AllowancePositionManager: creditDelegation": "46858",
"AllowancePositionManager: creditDelegationWithSig": "63046",
"AllowancePositionManager: renounceCreditDelegation": "24989",
"AllowancePositionManager: renounceWithdrawAllowance": "24900",
"AllowancePositionManager: withdrawOnBehalfOf: full": "123417",
"AllowancePositionManager: withdrawOnBehalfOf: partial": "134071",
"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",
Expand Down
130 changes: 112 additions & 18 deletions src/position-manager/AllowancePositionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 & ~0ff for collision resistance :gigachad:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, I'm not convinced what values these bring. Curious to understand other's thoughts as well

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they bring collision resistance from potentially using this slot (which we clearly don't here) & are generally considered best practice

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do -1 & ~0xff everywhere, these measures do not bring any value: we will still have a collision if same string is used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why would we do it everywhere? we just it exceptionally here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea of this was not a best practice to use strings + mappings, its just to bring additional randomness on slot derivation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why would we do it everywhere?

if we follow this pattern (-1 & ~0xff) everywhere, then collisions are still guaranteed when the same underlying string is used.

its just to bring additional randomness on slot derivation

additional randomness relative to what?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to potentially using a string for slot derivation, we dont know but its just good practice.
'if we follow this pattern (-1 & ~0xff) everywhere' ==> see we wouldn't use it everywhere, just here
oz also does

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


/// @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_) {}
Expand All @@ -58,6 +72,11 @@ contract AllowancePositionManager is
_updateWithdrawAllowance(params.owner, params.spender, params.reserveId, params.amount, true);
}

/// @inheritdoc IAllowancePositionManager
function temporaryApproveWithdraw(address spender, uint256 reserveId, uint256 amount) external {
_temporaryWithdrawAllowancesSlot(msg.sender, spender, reserveId).tstore(amount);
}

/// @inheritdoc IAllowancePositionManager
function creditDelegation(address spender, uint256 reserveId, uint256 amount) external {
_updateCreditDelegation(msg.sender, spender, reserveId, amount, true);
Expand All @@ -79,6 +98,11 @@ contract AllowancePositionManager is
_updateCreditDelegation(params.owner, params.spender, params.reserveId, params.amount, true);
}

/// @inheritdoc IAllowancePositionManager
function temporaryCreditDelegation(address spender, uint256 reserveId, uint256 amount) external {
_temporaryCreditDelegationsSlot(msg.sender, spender, reserveId).tstore(amount);
}

/// @inheritdoc IAllowancePositionManager
function renounceWithdrawAllowance(address owner, uint256 reserveId) external {
_updateWithdrawAllowance(
Expand Down Expand Up @@ -108,15 +132,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));
_updateWithdrawAllowance(
onBehalfOf,
msg.sender,
reserveId,
currentAllowance.uncheckedSub(amount),
true
);
_spendWithdrawAllowance(onBehalfOf, msg.sender, reserveId, amount);

IERC20 asset = _getReserveUnderlying(reserveId);
(uint256 withdrawnShares, uint256 withdrawnAmount) = ISpokeBase(SPOKE).withdraw(
Expand All @@ -136,15 +152,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));
_updateCreditDelegation(
onBehalfOf,
msg.sender,
reserveId,
currentAllowance.uncheckedSub(amount),
true
);
_spendCreditDelegation(onBehalfOf, msg.sender, reserveId, amount);

IERC20 asset = _getReserveUnderlying(reserveId);
(uint256 borrowedShares, uint256 borrowedAmount) = ISpokeBase(SPOKE).borrow(
Expand Down Expand Up @@ -219,4 +227,90 @@ 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));
_updateWithdrawAllowance({
owner: onBehalfOf,
spender: spender,
reserveId: reserveId,
newAllowance: allowance.uncheckedSub(amount),
emitEvent: true
});
}
}

/// @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));
_updateCreditDelegation({
owner: onBehalfOf,
spender: spender,
reserveId: reserveId,
newCreditDelegation: allowance.uncheckedSub(amount),
emitEvent: true
});
}
}

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();
}
}
14 changes: 14 additions & 0 deletions src/position-manager/interfaces/IAllowancePositionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -58,6 +65,13 @@ 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 temporaryCreditDelegation(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.
Expand Down
61 changes: 61 additions & 0 deletions tests/gas/PositionManagers.Operations.gas.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down
28 changes: 28 additions & 0 deletions tests/mocks/AllowancePositionManagerWrapper.sol
Original file line number Diff line number Diff line change
@@ -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();
}
}
Loading
Loading