-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Product lines basic #70
Changes from all commits
db6ff34
32f87b0
77c6a69
715e098
073e498
34d74ce
2c24319
d2aede1
a2f23b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
// SPDX-License-Identifier: GPL-2.0-or-later | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import {IERC20, IEVault, IGovernance} from "../EVault/IEVault.sol"; | ||
import {GenericFactory} from "../GenericFactory/GenericFactory.sol"; | ||
import {RevertBytes} from "../EVault/shared/lib/RevertBytes.sol"; | ||
|
||
import "../EVault/shared/Constants.sol"; | ||
|
||
/// @notice Base contract for product line contracts, which deploy pre-configured EVaults through a GenericFactory | ||
abstract contract BaseProductLine { | ||
// Constants | ||
|
||
uint256 constant REENTRANCYLOCK__UNLOCKED = 1; | ||
uint256 constant REENTRANCYLOCK__LOCKED = 2; | ||
|
||
address public immutable vaultFactory; | ||
address public immutable evc; | ||
|
||
// State | ||
|
||
uint256 private reentrancyLock; | ||
|
||
mapping(address vault => bool created) public vaultLookup; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can also be taken from OZ - enumerable set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as reentrancy test - either use the imports everywhere or not. The set is an overkill for the needs here and we'll still need the getters.. |
||
address[] public vaultList; | ||
|
||
// Events | ||
|
||
event Genesis(); | ||
event VaultCreated(address indexed vault, address indexed asset, bool upgradeable); | ||
|
||
// Errors | ||
|
||
error E_Reentrancy(); | ||
error E_BadQuery(); | ||
|
||
// Modifiers | ||
|
||
modifier nonReentrant() { | ||
if (reentrancyLock != REENTRANCYLOCK__UNLOCKED) revert E_Reentrancy(); | ||
|
||
reentrancyLock = REENTRANCYLOCK__LOCKED; | ||
_; | ||
reentrancyLock = REENTRANCYLOCK__UNLOCKED; | ||
} | ||
|
||
// Interface | ||
|
||
constructor(address vaultFactory_, address evc_) { | ||
vaultFactory = vaultFactory_; | ||
evc = evc_; | ||
|
||
emit Genesis(); | ||
} | ||
|
||
function makeNewVaultInternal(bool upgradeable, address asset, address oracle, address unitOfAccount) | ||
internal | ||
returns (IEVault) | ||
{ | ||
address newVault = | ||
GenericFactory(vaultFactory).createProxy(upgradeable, abi.encodePacked(asset, oracle, unitOfAccount)); | ||
|
||
vaultLookup[newVault] = true; | ||
vaultList.push(newVault); | ||
|
||
if (isEVCCompatible(asset)) { | ||
uint32 flags = IEVault(newVault).configFlags(); | ||
IEVault(newVault).setConfigFlags(flags | CFG_EVC_COMPATIBLE_ASSET); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see it is handled in the functions that inherits this one 👍🏽 |
||
} | ||
|
||
emit VaultCreated(newVault, asset, upgradeable); | ||
|
||
return IEVault(newVault); | ||
} | ||
|
||
// Getters | ||
|
||
function getVaultListLength() external view returns (uint256) { | ||
return vaultList.length; | ||
} | ||
|
||
function getVaultListSlice(uint256 start, uint256 end) external view returns (address[] memory list) { | ||
if (end == type(uint256).max) end = vaultList.length; | ||
if (end < start || end > vaultList.length) revert E_BadQuery(); | ||
|
||
list = new address[](end - start); | ||
for (uint256 i; i < end - start; ++i) { | ||
list[i] = vaultList[start + i]; | ||
} | ||
} | ||
|
||
function getTokenName(address asset) internal view returns (string memory) { | ||
// Handle MKR like tokens returning bytes32 | ||
(bool success, bytes memory data) = address(asset).staticcall(abi.encodeWithSelector(IERC20.name.selector)); | ||
if (!success) RevertBytes.revertBytes(data); | ||
return data.length == 32 ? string(data) : abi.decode(data, (string)); | ||
} | ||
|
||
function getTokenSymbol(address asset) internal view returns (string memory) { | ||
// Handle MKR like tokens returning bytes32 | ||
(bool success, bytes memory data) = address(asset).staticcall(abi.encodeWithSelector(IERC20.symbol.selector)); | ||
if (!success) RevertBytes.revertBytes(data); | ||
return data.length == 32 ? string(data) : abi.decode(data, (string)); | ||
} | ||
|
||
function isEVCCompatible(address asset) private view returns (bool) { | ||
(bool success, bytes memory data) = asset.staticcall(abi.encodeCall(IGovernance.EVC, ())); | ||
return success && data.length >= 32 && abi.decode(data, (address)) == address(evc); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
// SPDX-License-Identifier: GPL-2.0-or-later | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import "./BaseProductLine.sol"; | ||
|
||
/// @notice Contract deploying EVaults, forming the `Core` product line, which are upgradeable and fully governed. | ||
contract Core is BaseProductLine { | ||
// Constants | ||
|
||
bool public constant UPGRADEABLE = true; | ||
|
||
// State | ||
|
||
address public governor; | ||
address public feeReceiver; | ||
Comment on lines
+15
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we have functions that allow to set those addresses by governor? |
||
|
||
// Errors | ||
|
||
error E_Unauthorized(); | ||
|
||
// Interface | ||
|
||
constructor(address vaultFactory_, address evc_, address governor_, address feeReceiver_) | ||
BaseProductLine(vaultFactory_, evc_) | ||
{ | ||
governor = governor_; | ||
feeReceiver = feeReceiver_; | ||
} | ||
|
||
modifier governorOnly() { | ||
if (msg.sender != governor) revert E_Unauthorized(); | ||
_; | ||
} | ||
|
||
function createVault(address asset, address oracle, address unitOfAccount) | ||
external | ||
governorOnly | ||
returns (address) | ||
{ | ||
IEVault vault = makeNewVaultInternal(UPGRADEABLE, asset, oracle, unitOfAccount); | ||
|
||
vault.setName(string.concat("Core vault: ", getTokenName(asset))); | ||
vault.setSymbol(string.concat("e", getTokenSymbol(asset))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. symbol must also encode the product line I think. otherwise there's no way to distinguish between two vaults of the same asset by looking at the symbol There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. even within a product line there could be duplicate assets. In core, the governor can simply change the name. Escrow does enforce single asset per line. How about But what about Edge vaults? edgUSDCn? where n is a counter of USDC vaults? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point re multiple vaults of the same asset. the UI assumes that vaults names and symbols also have numbers. I think without them, it will be harder to handle for FE team. why don't we have a counter for each asset and add a number to the name and symbol? i.e. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Product lines will be developed further for sure. Let's maybe keep these improvements in mind for further work? For now we want yAudit to get a sense of what these things are |
||
|
||
vault.setFeeReceiver(feeReceiver); | ||
vault.setGovernorAdmin(governor); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about IRM? we need to set something default which should probably be stored as immutable. btw, I noticed that we have inconsistency in Governance module: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the default value for IRM is address(0) which is handled as 0% interest set. Good point about the setter There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll fix the setter straight on master There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I know and I think it's problematic. when a new borrowable vault enabled, it should at least have a default interest rate installed. similar to what we had in Euler v1 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A new Core vault won't have any LTV's either, so its not usable without further config anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we'll probably need some default set of collaterals that gets set up of every new core vault |
||
|
||
return address(vault); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
// SPDX-License-Identifier: GPL-2.0-or-later | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import "./BaseProductLine.sol"; | ||
import "../EVault/shared/Constants.sol"; | ||
|
||
/// @notice Contract deploying EVaults, forming the `Escrow` product line, which are non-upgradeable | ||
/// non-governed, don't allow borrowing and only allow one instance per asset. | ||
contract Escrow is BaseProductLine { | ||
// Constants | ||
|
||
bool public constant UPGRADEABLE = false; | ||
|
||
// State | ||
|
||
mapping(address asset => address vault) public assetLookup; | ||
|
||
// Errors | ||
|
||
error E_AlreadyCreated(); | ||
|
||
// Interface | ||
|
||
constructor(address vaultFactory_, address evc_) BaseProductLine(vaultFactory_, evc_) {} | ||
|
||
function createVault(address asset) external returns (address) { | ||
if (assetLookup[asset] != address(0)) revert E_AlreadyCreated(); | ||
|
||
IEVault vault = makeNewVaultInternal(UPGRADEABLE, asset, address(0), address(0)); | ||
|
||
assetLookup[asset] = address(vault); | ||
|
||
vault.setName(string.concat("Escrow vault: ", getTokenName(asset))); | ||
vault.setSymbol(string.concat("e", getTokenSymbol(asset))); | ||
|
||
vault.setDisabledOps( | ||
OP_BORROW | OP_REPAY | OP_LOOP | OP_DELOOP | OP_PULL_DEBT | OP_CONVERT_FEES | OP_LIQUIDATE | OP_TOUCH | ||
| OP_ACCRUE_INTEREST | ||
); | ||
|
||
// Renounce governorship | ||
vault.setGovernorAdmin(address(0)); | ||
|
||
return address(vault); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// SPDX-License-Identifier: GPL-2.0-or-later | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import "../evault/EVaultTestBase.t.sol"; | ||
|
||
contract ProductLine_Base is EVaultTestBase { | ||
function test_ProductLine_Base_lookup() public view { | ||
assertEq(coreProductLine.vaultLookup(address(eTST)), true); | ||
assertEq(coreProductLine.vaultLookup(vm.addr(100)), false); | ||
assertEq(coreProductLine.getVaultListLength(), 2); | ||
assertEq(coreProductLine.getVaultListSlice(0, type(uint256).max)[0], address(eTST)); | ||
assertEq(coreProductLine.getVaultListSlice(0, type(uint256).max)[1], address(eTST2)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
// SPDX-License-Identifier: GPL-2.0-or-later | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import "../evault/EVaultTestBase.t.sol"; | ||
|
||
contract ProductLine_Core is EVaultTestBase { | ||
function test_ProductLine_Core_basicViews() public view { | ||
assertEq(factory.getProxyConfig(address(eTST)).upgradeable, true); | ||
|
||
assertEq(eTST.unitOfAccount(), unitOfAccount); | ||
assertEq(eTST.oracle(), address(oracle)); | ||
assertEq(eTST.feeReceiver(), feeReceiver); | ||
assertEq(eTST.governorAdmin(), address(this)); | ||
} | ||
|
||
function test_ProductLine_Core_EVCCompatibility() public { | ||
assertEq(eTST.configFlags(), 0); | ||
IEVault nested = IEVault(coreProductLine.createVault(address(eTST), address(oracle), unitOfAccount)); | ||
assertEq(nested.configFlags(), CFG_EVC_COMPATIBLE_ASSET); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
// SPDX-License-Identifier: GPL-2.0-or-later | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import "src/ProductLines/Escrow.sol"; | ||
import "../evault/EVaultTestBase.t.sol"; | ||
|
||
contract ProductLine_Escrow is EVaultTestBase { | ||
uint32 constant ESCROW_DISABLED_OPS = OP_BORROW | OP_REPAY | OP_LOOP | OP_DELOOP | OP_PULL_DEBT | OP_CONVERT_FEES | ||
| OP_LIQUIDATE | OP_TOUCH | OP_ACCRUE_INTEREST; | ||
|
||
function test_ProductLine_Escrow_basicViews() public { | ||
IEVault escrowTST = IEVault(escrowProductLine.createVault(address(assetTST))); | ||
|
||
assertEq(factory.getProxyConfig(address(escrowTST)).upgradeable, false); | ||
|
||
assertEq(escrowTST.name(), "Escrow vault: Test Token"); | ||
assertEq(escrowTST.symbol(), "eTST"); | ||
assertEq(escrowTST.unitOfAccount(), address(0)); | ||
assertEq(escrowTST.oracle(), address(0)); | ||
assertEq(escrowTST.disabledOps(), ESCROW_DISABLED_OPS); | ||
} | ||
|
||
function test_ProductLine_Escrow_RevertWhenAlreadyCreated() public { | ||
escrowProductLine.createVault(address(assetTST)); | ||
|
||
vm.expectRevert(Escrow.E_AlreadyCreated.selector); | ||
escrowProductLine.createVault(address(assetTST)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's inherit from OZ maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we would decide on the import policy. We didn't use to do it at all, but now the synths are... but not the reentrancy lock.. Leave it for consistency? Or do it in the synths as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we discussed that eventually synths will be moved to a separate repo. I think we decided to do the same for the product lines, right? I think in repos outside EVK we should be using external dependencies freely