Skip to content

feat: add permissioned borrow access manager - #1336

Draft
avniculae wants to merge 3 commits into
mainfrom
feat/permissioned-borrow-access-manager
Draft

feat: add permissioned borrow access manager#1336
avniculae wants to merge 3 commits into
mainfrom
feat/permissioned-borrow-access-manager

Conversation

@avniculae

Copy link
Copy Markdown
Contributor

No description provided.

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

}

/// @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

Comment thread src/spoke/Spoke.sol
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

Comment thread src/spoke/Spoke.sol
}

/// @dev To be overridden by the inheriting Spoke instance contract.
function initialize(address authority) external virtual;

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.

upside is that by just updating the authority via setAuthority we can modify general access control
no upgrade required, no separate implemenation for restricting access

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants