Basic Implementation of Single Asset Vault #1
Conversation
| _grantRole(DEFAULT_ADMIN_ROLE, admin_); | ||
| _grantRole(OPERATOR_ROLE, operator_); | ||
| } | ||
|
|
There was a problem hiding this comment.
if we're to make the vault work with
karak
symbiotic
whatever kernel does in the future
we need to override totalAssets() and calculate things with something like the TVL modules in mellow:
https://github.com/mellow-finance/mellow-lrt/blob/main/src/modules/erc20/ERC20TvlModule.sol
https://github.com/mellow-finance/mellow-lrt/blob/main/src/modules/erc20/ERC20TvlModule.sol
We could launch this initial version without it and then make a v2 with this type of functionality.
There was a problem hiding this comment.
i would experiment with a prototype now on a prototype-branch (doesn't need to go into audit) just to validate quickly that approach would work well with karak and symbiotic.
Given that symbiotic already works on mellow , i expect it just works
There was a problem hiding this comment.
This is a SingleAsset vault. Would the tvl not be 1 to 1 with the underlying asset?
src/SingleVault.sol
Outdated
| address[] memory proposers_, | ||
| address[] memory executors_ | ||
| ) internal pure { | ||
| require(asset_ != IERC20(address(0)), "Asset cannot be zero address"); |
There was a problem hiding this comment.
we should use named errors like ZeroAddress()
| constructor() { | ||
| _disableInitializers(); | ||
| } | ||
|
|
There was a problem hiding this comment.
product question: do we want to be able to puase withdrawals?
There was a problem hiding this comment.
I think we should not pause withdraws for this version of the vault.
There was a problem hiding this comment.
@danoctavian This vault being as plain vanilla OZ as possible is safer for the expedited timeline for BSC Kernel.
| IERC20 | ||
| } from "src/Common.sol"; | ||
|
|
||
| contract VaultFactory is AccessControlUpgradeable { |
There was a problem hiding this comment.
thinking what's the value of the factory beyond cheking for symbol duplicates now
One thing is the vaults as they are are vulnerable to donation attacks because of how ERC4626Upgradeable is built
So we can consider boostrapping all vaults at creation time in the factory.
There was a problem hiding this comment.
Having a factory makes it easier to deploy a Vault
There was a problem hiding this comment.
@danoctavian Thank you. I added a bootstrap to the factory.
Also note that as of v5 of ERC4626 Upgradeable, OZ claims to have mitigated the donation attack.
This is something we can point to for any bug bounty requests.
|
|
||
| import {ISingleVault} from "src/ISingleVault.sol"; | ||
|
|
||
| contract SingleVault is ISingleVault, ERC4626Upgradeable, TimelockControllerUpgradeable, ReentrancyGuardUpgradeable { |
There was a problem hiding this comment.
with regards to the whitelistTx functionality:
Just whileisting txdata blob is too simple to be useful
We'd need to be able to parse out the selector and check the target and against. a whitelist to make it useful to validate in a general way things like:
someSymbioticVault.deposit(amount, receiver)
something akin to this https://github.com/mellow-finance/mellow-lrt/blob/main/src/validators/DefaultBondValidator.sol
There was a problem hiding this comment.
Considering how we engaged Zokyo for delivery of code for yesterday, Module integration will have to wait until after this initial version.
There was a problem hiding this comment.
@danoctavian I really like the modular design of the mellow vaults.
This is a basic implementation of the vault.
Features: