Add a vault to store ActivePool assets#6
Conversation
| event StabilityPoolAddressChanged(address _newStabilityPoolAddress); | ||
| event ActivePoolBoldDebtUpdated(uint256 _recordedDebtSum); | ||
| event ActivePoolCollBalanceUpdated(uint256 _collBalance); | ||
| event CollateralVaultChanged(address _newCollateralVault); |
There was a problem hiding this comment.
There is no setter method for vault to have this event emitted.
| interestRouter = _addressesRegistry.interestRouter(); | ||
| boldToken = _addressesRegistry.boldToken(); | ||
| parameters = _addressesRegistry.parameters(); | ||
| vault = address(_addressesRegistry.collateralVault()); |
There was a problem hiding this comment.
I believe, let's not add collateralVault's address to addressesRegistry as collateralVault feels like a dynamic address or more likely to be changed frequently compared to core contracts. The collateralVault should be get-able directly from ActivePool's interface via a getter method
| _accountForSendColl(_amount); | ||
|
|
||
| collToken.safeTransfer(_account, _amount); | ||
| uint256 b = collToken.balanceOf(address(vault)); |
There was a problem hiding this comment.
Didn't understand the purpose of uint256 a and uint256 b
| function _pullCollAndSendToActivePool(IActivePool _activePool, uint256 _amount) internal { | ||
| // Send Coll tokens from sender to active pool | ||
| collToken.safeTransferFrom(msg.sender, address(_activePool), _amount); | ||
| collToken.safeTransferFrom(msg.sender, address(_activePool.vault()), _amount); |
There was a problem hiding this comment.
So, _activePool.vault() should be the correct way of fetching the collateralVault. Should be removed from addressesRegistry.
| collToken = _addressesRegistry.collToken(); | ||
| troveManagerAddress = address(_addressesRegistry.troveManager()); | ||
| activePoolAddress = address(_addressesRegistry.activePool()); | ||
| activePoolVaultAddress = address(_addressesRegistry.collateralVault()); |
There was a problem hiding this comment.
Caching activePoolVaultAddress in multiple contracts requires multiple setters. I recommend fetching the address directly from activePool to avoid this redundancy. Also, the naming for the external vault is inconsistent across contracts
| import "./Interfaces/IAddressesRegistry.sol"; | ||
| import "./Interfaces/ICollateralVault.sol"; | ||
|
|
||
| contract CollateralVault is Ownable2StepUpgradeable, ICollateralVault { |
There was a problem hiding this comment.
I kinda recommend standard EIP4626 vault or async vault instead of custom one to avoid integration issues if we create strategies later on for yield
2 participants
No description provided.