Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
52 changes: 52 additions & 0 deletions src/access/PermissionedBorrowAccessManager.sol

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

can you please add gas snapshots for using this access manager ? I suspect this adds ~5-10k gas costs

Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// SPDX-License-Identifier: LicenseRef-BUSL
pragma solidity 0.8.28;

import {AccessManagerEnumerable} from 'src/access/AccessManagerEnumerable.sol';
import {IBorrowerEligibility} from 'src/access/interfaces/IBorrowerEligibility.sol';
import {ISpoke} from 'src/spoke/interfaces/ISpoke.sol';

/// @title PermissionedBorrowAccessManager
/// @author Aave Labs
/// @notice Contextual access manager that restricts borrowing to eligible position owners.
/// @dev Non-borrow position actions retain the Spoke's standard position-manager authorization.
/// Explicit AccessManager roles can authorize callers before this custom policy is evaluated.
contract PermissionedBorrowAccessManager is AccessManagerEnumerable {
/// @notice The Spoke controlled by this access manager.
ISpoke public immutable SPOKE;

/// @notice The provider used to determine borrower eligibility.
IBorrowerEligibility public immutable BORROWER_ELIGIBILITY;

/// @dev Constructor.
/// @param initialAdmin_ The address of the initial admin.
/// @param spoke_ The Spoke controlled by this access manager.
/// @param borrowerEligibility_ The provider used to determine borrower eligibility.
constructor(
address initialAdmin_,
ISpoke spoke_,
IBorrowerEligibility borrowerEligibility_
) AccessManagerEnumerable(initialAdmin_) {
require(address(spoke_) != address(0), ISpoke.InvalidAddress());
require(address(borrowerEligibility_) != address(0), ISpoke.InvalidAddress());
SPOKE = spoke_;
BORROWER_ELIGIBILITY = borrowerEligibility_;
}

/// @dev Extends the default position-manager policy with borrower eligibility.
function _isPositionActionAllowed(
address caller,
address target,
bytes calldata data
) internal view override returns (bool handled, bool allowed) {
if (target != address(SPOKE)) return super._isPositionActionAllowed(caller, target, data);

(bool valid, address onBehalfOf) = _decodePositionAction(data);
if (!valid) return (true, false);

(, allowed) = super._isPositionActionAllowed(caller, target, data);
if (!allowed) return (true, false);

allowed = bytes4(data) != ISpoke.borrow.selector || BORROWER_ELIGIBILITY.isEligible(onBehalfOf);
return (true, allowed);
}
}
10 changes: 10 additions & 0 deletions src/access/interfaces/IBorrowerEligibility.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// SPDX-License-Identifier: LicenseRef-BUSL
pragma solidity ^0.8.0;

/// @title IBorrowerEligibility
/// @author Aave Labs
/// @notice Interface for a provider of permissioned-borrowing eligibility.
interface IBorrowerEligibility {
/// @notice Returns whether `account` is eligible to borrow.
function isEligible(address account) external view returns (bool);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

pragma solidity ^0.8.20;

import {AuthorityUtils} from '../openzeppelin/AuthorityUtils.sol';
import {IAccessManager} from '../openzeppelin/IAccessManager.sol';
import {IAccessManaged} from '../openzeppelin/IAccessManaged.sol';
import {ContextUpgradeable} from './ContextUpgradeable.sol';
Expand Down Expand Up @@ -114,11 +113,10 @@ abstract contract AccessManagedUpgradeable is Initializable, ContextUpgradeable,
*/
function _checkCanCall(address caller, bytes calldata data) internal virtual {
AccessManagedStorage storage $ = _getAccessManagedStorage();
(bool immediate, uint32 delay) = AuthorityUtils.canCallWithDelay(
authority(),
(bool immediate, uint32 delay) = IAccessManager(authority()).canCall(
caller,
address(this),
bytes4(data[0:4])
data
);
if (!immediate) {
if (delay > 0) {
Expand Down
57 changes: 57 additions & 0 deletions src/dependencies/openzeppelin/AccessManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {Multicall} from './Multicall.sol';
import {Math} from './Math.sol';
import {Time} from './Time.sol';
import {Hashes} from './Hashes.sol';
import {ISpoke} from 'src/spoke/interfaces/ISpoke.sol';

/**
* @dev AccessManager is a central contract to store the permissions of a system.
Expand Down Expand Up @@ -155,6 +156,62 @@ contract AccessManager is Context, Multicall, IAccessManager {
}
}

/// @inheritdoc IAccessManager
function canCall(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I personally don't like the complexity of mantaining a modified OZ Access Manager

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

canCall is virtual so we can just add custom logic in PermissionedBorrowAccessManager without touching OZ deps no? @AlbertoCentonze
smt like

function canCall() override {
  if target and selector is spoke and borrow(), then call custom logic
  else super.canCall()
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we can keep the onlyPositionManager as is, and only add restricted

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We need a method overload that uses bytes instead bytes4 but yes technical we can just build on top. It still a custom implementation technically

address caller,
address target,
bytes calldata data
) public view virtual returns (bool immediate, uint32 delay) {
if (data.length < 4) return (false, 0);

if (_isPositionAction(bytes4(data))) {
(bool handled, bool allowed) = _isPositionActionAllowed(caller, target, data);
if (handled) return (allowed, 0);
}

return canCall(caller, target, bytes4(data));
}

/// @notice Returns whether a position action is allowed by its contextual policy.
/// @dev The default policy preserves the Spoke's position-manager authorization.
function _isPositionActionAllowed(
address caller,
address target,
bytes calldata data
) internal view virtual returns (bool handled, bool allowed) {
(bool valid, address onBehalfOf) = _decodePositionAction(data);
if (!valid) return (false, false);

(bool success, bytes memory result) = target.staticcall(
abi.encodeCall(ISpoke.isPositionManager, (onBehalfOf, caller))
);
if (!success || result.length != 32) return (false, false);
return (true, abi.decode(result, (bool)));
}

/// @notice Decodes the position owner from supported position-action calldata.
function _decodePositionAction(
bytes calldata data
) internal pure returns (bool valid, address onBehalfOf) {
if (data.length != 100 || !_isPositionAction(bytes4(data))) return (false, address(0));

uint256 encodedOnBehalfOf;
assembly ('memory-safe') {
encodedOnBehalfOf := calldataload(add(data.offset, 68))
}
if (encodedOnBehalfOf > type(uint160).max) return (false, address(0));
return (true, address(uint160(encodedOnBehalfOf)));
}

function _isPositionAction(bytes4 selector) internal pure returns (bool) {
return
selector == ISpoke.supply.selector ||
selector == ISpoke.withdraw.selector ||
selector == ISpoke.borrow.selector ||
selector == ISpoke.repay.selector ||
selector == ISpoke.setUsingAsCollateral.selector;
}

/// @inheritdoc IAccessManager
function expiration() public view virtual returns (uint32) {
return 1 weeks;
Expand Down
11 changes: 11 additions & 0 deletions src/dependencies/openzeppelin/IAccessManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,17 @@ interface IAccessManager {
bytes4 selector
) external view returns (bool allowed, uint32 delay);

/**
* @dev Contextual overload of {canCall} that receives the complete target calldata.
* The default AccessManager implementation authorizes by selector, while derived managers may
* additionally inspect call arguments.
*/
function canCall(
address caller,
address target,
bytes calldata data
) external view returns (bool allowed, uint32 delay);

/**
* @dev Expiration delay for scheduled proposals. Defaults to 1 week.
*
Expand Down
16 changes: 5 additions & 11 deletions src/spoke/Spoke.sol
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,6 @@ abstract contract Spoke is
uint256 internal constant DUST_LIQUIDATION_THRESHOLD =
LiquidationLogic.DUST_LIQUIDATION_THRESHOLD;

/// @notice Modifier that checks if the caller is an approved positionManager for `onBehalfOf`.
modifier onlyPositionManager(address onBehalfOf) {
require(_isPositionManager({user: onBehalfOf, manager: msg.sender}), Unauthorized());
_;
}

/// @dev Constructor.
/// @param oracle_ The address of the AaveOracle contract.
/// @param maxUserReservesLimit_ The maximum number of collateral and borrow reserves a user can have.
Expand Down Expand Up @@ -226,7 +220,7 @@ abstract contract Spoke is
uint256 reserveId,
uint256 amount,
address onBehalfOf
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
) external nonReentrant restricted returns (uint256, uint256) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

one advantage of this is that we can reuse the same spoke impl for canonical and permissioned/horizon spokes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is because we are moving the onlyPositionManager to the access manager. If we don't do it, we may need a seaprate implementation anyway for collateral seizing

Reserve storage reserve = _reserves.get(reserveId);
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
_validateSupply(reserve.flags);
Expand All @@ -245,7 +239,7 @@ abstract contract Spoke is
uint256 reserveId,
uint256 amount,
address onBehalfOf
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
) external nonReentrant restricted returns (uint256, uint256) {
Reserve storage reserve = _reserves.get(reserveId);
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
_validateWithdraw(reserve.flags);
Expand Down Expand Up @@ -275,7 +269,7 @@ abstract contract Spoke is
uint256 reserveId,
uint256 amount,
address onBehalfOf
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
) external nonReentrant restricted returns (uint256, uint256) {
Reserve storage reserve = _reserves.get(reserveId);
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
PositionStatus storage positionStatus = _positionStatus[onBehalfOf];
Expand Down Expand Up @@ -306,7 +300,7 @@ abstract contract Spoke is
uint256 reserveId,
uint256 amount,
address onBehalfOf
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
) external nonReentrant restricted returns (uint256, uint256) {
Reserve storage reserve = _reserves.get(reserveId);
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
_validateRepay(reserve.flags);
Expand Down Expand Up @@ -392,7 +386,7 @@ abstract contract Spoke is
uint256 reserveId,
bool usingAsCollateral,
address onBehalfOf
) external nonReentrant onlyPositionManager(onBehalfOf) {
) external nonReentrant restricted {
Reserve storage reserve = _reserves.get(reserveId);
PositionStatus storage positionStatus = _positionStatus[onBehalfOf];
if (positionStatus.isUsingAsCollateral(reserveId) == usingAsCollateral) {
Expand Down
Loading
Loading