V2 Vault Infra Contracts Redesign

[mayorcoded]

Having redesigned our strategies and vaults, we are opening this issue to explore the possibilities of resigning the infrastructure contracts that deploy our vaults. These are the contracts that are currently being considered for a redesign:

AdminProxy.sol FeeRecipientProxy.sol CloneRegistry.sol TemplateRegistry.sol PermissionRegistry.sol CloneFactory.sol DeploymentController.sol VaultRegistry.sol VaultController.sol

The design considerations we are considering for these contracts are highlight below:

1. Deploying v2 vaults and strategies with the current infrastructure contracts
2. Calling management functions in v2 vaults and strategies with the current infrastructure contracts

To track the conversations around this issue, recommendations will be added as comments.

[mayorcoded]

Here are some of the design changes that we might consider making:

VaultController:

• The VaultInitParams in the deployVault function does not reflect the changes that we have made to our v2 base vault contracts, therefore we need to replace it with the new BaseVaultConfig struct.
• We might need to decouple the strategy/adapter deployment logic from the vault deployment logic as we currently have it. Based on the user experience of the VCI, a strategy is supposed to exist and be available for selection before a vault can be deployed, therefore we the vault controller should check that a strategy already exist before deploying or vault, or revert.
• Removing the staking management logic into a separate contract might make the vault controller simpler.
• The pausing logic need to be slightly modified to work with the new strategy design we have. For example, we need to replace IVault(vaults[i]).adapter() with IVault(vaults[i]).strategy(). Additionally, the access level on management functions like pausing logic need to be reviewed. We currently protect pausing functions with the onlyOwner guard, but the owner of a vault may not necessarily be the owner of the vault controller.
• Management logic like proposeVaultAdapters and changeVaultAdapters will be removed because the current base vault design does not support changing strategies.

Questions

• I see a staking deployment logic in the deployVault function, is this related to vePOP @RedVeil? If it is, could you provide mode details about how it works, please?

[RedVeil]

Thanks for the overview. Makes sense that we need to adjust the deploy logic.

In regards to your question. These staking contracts were originally build to allow for claiming rewards of the underlying adapter while simultaneously allowing anyone to incenitivise vaults. So they would always be deployed with each Vault. Didn't turn out like that in the end.

Btw. Can we call setRewardToken and similar admin functions on the strategies? Since all contracts are owned by the adminProxy I think we would need to add this to the controller.

[mayorcoded]

@RedVeil does that mean the staking logic would be removed since it didn't end up being used as imagined? We should be able to call admin functions on strategies.

[0xruhum]

Staking stuff should be removed. It clashes with the vePOP stuff.

[RedVeil]

yup what @0xruhum said

[mayorcoded]

@0xruhum can you provide some more clarification on how the vePOP integrates with our v2 vaults now? My reason for asking is that there is a VaultRouter.sol in v1 that practically takes some underlying asset and deposits it into a a vault and then stakes the shares received from the deposit. Will this logic (VaultRouter) persist for the v2 vaults or are we removing it altogether?

[0xruhum]

In the vePOP system, each vault has a gauge. You deposit your vault shares into that gauge to earn oPOP rewards. So the VaultRouter will do the same thing as before but instead of staking into the staking contract it'll deposit the vault shares into the gauge.

[mayorcoded]

Alright, that clears it up for me, thanks!

[0xruhum]

VaultController

• need to update template version
• vault & strategy deployment should be handled in separate functions
  • VaultController.sol
  • The existing function is bloated and hard to read
• initial deposit in deployVault() is unnecessary
  • VaultController.sol
  • The ERC4626 implementation by OZ uses virtual shares to mitigate the issue, see docs
• Management of the vault should be done through the vault's own contract instead of using the VaultController
  • pause, unpause, fee changes, etc.

VaultRegistry

• are swapTokenAddresses, swapAddress, exchange still relevant? staking is obselete.
  • L17-L21

CloneRegistry

• obselete now that we deploy a single version of each Strategy

CloneFactory

• we could keep it but there's no benefit in having a separate contract handle the deployment. The controller/factory contract could just do that

PermissionRegistry

• it allows us to endorse or reject tokens. So it's a whitelist for tokens for which you can create vaults.
• Is that whitelist still necessary or beneficial?

TemplateRegistry

• why did we choose to identify a template using a bytes32 hash instead of using the address?

Redesign

Contracts:

• VaultFactory
  • responsible for deploying new vaults
    • checks whether a given strategy is endorsed, i.e. part of the template registry
  • deploys the clone and adds it to the vault registry
    • no reason to move that to a separate contract
• VaultRegistry
  • stays as is but we change the metadata that we save.
• TemplateRegistry
  • stays as is
  • we could rename it to StrategyRegistry but the main functionality should stay the same.
• AdminProxy
  • stays as is to make ownership changes easier

[0xruhum]

How do we handle new versions?

• contracts that could be updated:
  • vaults
  • strategies
• the infra contracts should stay the same.

For each major version there's a separate namespace in the template registry contract, e.g. v1, v2, v3, etc.

Any vault registered for major version X is compatible with any strategy registered for the same version. Meaning, a v2 Vault can only use v2 strategies. In case a new major version is released, each strategy has to be evaluated on whether it's still compatible. If that's the case, it's added to the registry again for the new version.

For any minor or patch versions, we assume that they introduce non-breaking changes and thus are still compatible with any other strategy or vault in their respective major version. Thus, a vault with version v2.1 is compatible with a strategy with version v2.0.

This way we have a clear separation of which vault can interact with which strategy without mixing up different versions. Even if a v3 vault is compatible with a v2 strategy the factory shouldn't allow them to be deployed unless the same strategy is registered in the v3 namespace.

---

Given that requirement we have two options:

1. we have a single infra stack that handles deployments for each major version
2. each new version gets its own infra stack (registry & factory). Balancer does this for example. Each new major version has its own pool factory contract.

[mayorcoded]

@0xruhum @RedVeil please let me know if this diagram represents what we expect from the redesign

[0xruhum]

Sorry, forgot to reply here.

• The Deployment Controller will be removed in v2. Instead, we let the VaultFactory handle all of that. Also, we shouldn't use the deployment controller to deploy new strategies. They should just be deployed independently and then added to the system through the Template Registry. That way the deployment flow is equal for us and 3rd parties that also might develop strategies.

• The Vault Registry should only be callable by the Vault Factory. When a new vault is deployed, it will automatically add it to the registry.

• I personally don't see the appeal of the Permission Registry to endorse tokens. So I'd prefer to remove it.

• The Vault Factory should be owned by the AdminProxy and not by a custom owner (top of the graph).

[0xruhum]

TODOs for Vault Controller:

• [x] move clone factory logic into VaultController
  • the controller should handle deployments of vaults
• [x] remove vault admin logic from VaultController
  • all the vault configuration stuff should be handled through the Vault itself. This includes, but is not limited to, proposeVaultFees(), pauseVault(), etc
  • update the deployment logic to make the vault owner the vault creator or the address they pass for that role. It should not be the AdminProxy
• [x] remove the initial deposit logic from the vault deployment function
  • the new OZ contracts have protection built in to prevent share manipulation attacks. Thus we don't need the initial deposit anymore.
• [x] remove the deploy adapter and deploy strategy logic.