feat: use cloning for vaults #191
Conversation
|
Remind me, what's motivating the switch between blueprints and clones? Is there a negative tradeoff to counter the positive of lower deployment cost? |
The main trade off is higher runtime costs. Both from needing a delegatecall. But also mainly from not being able to use Immutables. |
| asset: ERC20, | ||
| def __init__(): | ||
| # Set `asset` so it cannot be re-initialized. | ||
| self.asset = self |
There was a problem hiding this comment.
does this need to call initialize?
There was a problem hiding this comment.
Assuming the first one deployed is just a dummy version to use for cloning.
So just want to make sure it can't be initialized ever and only used as the original
There was a problem hiding this comment.
wouldnt work correctly to use the original since msg.sender would not be the factory
| # Underlying token used by the vault. | ||
| ASSET: immutable(ERC20) |
There was a problem hiding this comment.
kind of sad losing immutables
There was a problem hiding this comment.
Agreed.
We will have to do some more research on the clones with immutables to see if it can be done in vyper
contracts/VaultV3.vy
Outdated
| # Deployer contract used to retrieve the protocol fee config. | ||
| FACTORY: public(immutable(address)) | ||
| FACTORY: public(address) |
There was a problem hiding this comment.
should this be lower cased as well?
There was a problem hiding this comment.
Technically yes, but the strategy's have it as upper case and same with the previous versions.
So kept it upper case to keep all the ABI's constant
ASSET and DECIMALS weren't public so it didnt matter for the ABI
There was a problem hiding this comment.
I guess the storage variable could be lower case and internal, then make a getter that matches for the ABI?
There was a problem hiding this comment.
Changed it to be lower case and add a separate getter 505e4df
* feat: update api and ape version * chore: removals (#189) * chore: remove increase and decrease allowances * chore: remove open roles * feat: add max loss to debt updates (#190) * feat: use cloning for vaults (#191) * feat: use cloning for vaults * fix: scripts * chore: fix interfaces * chore: lower case factory * fix: update to strategy changes * feat: add to queue flag (#195) * fix: updated strategy branch * feat: minor fixes (#194) * fix: redeem corrections * chore: dont burn zero shares * fix: use updated strategy storage * fix: rebase * chore: bump oz version * fix: oz 4626 fix * fix: lossy test * fix: round down in max redeem * fix: comment * chore: ignore snapshot * test: add foundry tests (#196) * chore: setup foundry test * chore: add remappings * forge install: erc4626-tests * test: add foundry fuzzing tests * fix: max uint deposit limit * fix: test strategy * fix: foundry runner * fix: clamp overflow * fix: default tests * chore: clean up linting * fix: new strategy version * fix: reporting costs (#192) * test: for no locking * chore: cheaper reports * chore: track current debt * chore: lower unlocked gas * fix: set to 0 * build: only burn or mint (#193) * build: only burn or mint * build: target end supply * chore: comments * fix: comments * fix: strategy changes * chore: rebase * fix: decimal type * chore: deployment and interfaces * fix: black * chore: updated strategy branch * fix: foundry remapping
Description
Switch from using blueprints to cloning for lower deployment costs.
Fixes # (issue)
Checklist