Skip to content

Conversation

@gnarvaja
Copy link
Contributor

@gnarvaja gnarvaja commented Jan 3, 2025

Previously the forwardToStrategy calls where open to anyone and the strategies implemented the access control, assuming the vault was an IAccessControl.

Now the strategies don't implement any access control logic, and this responsability is left to the vaults.

The MultiStrategyERC4626 implements the access control by requiring two roles, one generic (FORWARD_TO_STRATEGY_ROLE) and one specific of the strategy and method being called.

The AccessManagedMSV implements the access control by requiring from the access manager an authorization to call a fake method id (bytes4 selector) generated from the specific of the
strategy and method being called. Also the permission to call forwardToStrategy must be granted too.

Previously the forwardToStrategy calls where open to anyone and the
strategies implemented the access control, assuming the vault was an
IAccessControl.

Now the strategies don't implement any access control logic, and this
responsability is left to the vaults.

The MultiStrategyERC4626 implements the access control by requiring two
roles, one generic (FORWARD_TO_STRATEGY_ROLE) and one specific of the
strategy and method being called.

The AccessManagedMSV implements the access control by requiring from the
access manager an authorization to call a fake method id (bytes4
selector) generated from the specific of the
strategy and method being called. Also the permission to call
forwardToStrategy must be granted too.
We are not using this contract in production and has known errors. It
only makes sense for testing individual strategies.
Also, it uses customized deployProxy call available since OZ-upgrades
3.7.
The strategies no longer raise these kind of errors.
@gnarvaja gnarvaja merged commit 69dcbb6 into main Jan 3, 2025
2 checks passed
@gnarvaja gnarvaja deleted the fix-forward-to-strategy-access branch January 3, 2025 19:56
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.

2 participants