PCV Deposit V2 #181
PCV Deposit V2 #181
Conversation
…volt-protocol-core into feat/pcv-deposit-v2-base
…volt-protocol-core into feat/pcv-deposit-v2-base
eswak
left a comment
eswak
left a comment
There was a problem hiding this comment.
very cool that pcv deposits will be so easy to add with this refactoring. I think you should consider doing a standard test suite for PCVDeposits (and inherit it in your venue-specific test files)
requested changes because I don't like the duplication of token state variable + rewards handling, but overall very excited by this PR
we should consider changing VIP17 to deposit a small amount in each venue
| /// @notice claim COMP rewards for supplying to Morpho. | ||
| /// Does not require reentrancy lock as no smart contract state is mutated | ||
| /// in this function. |
| /// @notice gets the token address in which this deposit returns its balance | ||
| function balanceReportedIn() external view returns (address); | ||
|
|
||
| /// @notice address of underlying token | ||
| function token() external view returns (address); |
| using SafeERC20 for IERC20; | ||
| using SafeCast for *; | ||
|
|
||
| /// @notice reference to underlying token |
| import {CoreRefV2} from "@voltprotocol/refs/CoreRefV2.sol"; | ||
| import {IPCVDepositV2} from "@voltprotocol/pcv/IPCVDepositV2.sol"; | ||
|
|
||
| /// @title abstract contract for withdrawing ERC-20 tokens using a PCV Controller |
| ); | ||
| } | ||
|
|
||
| function testWithdrawPCVGuardian() public { |
There was a problem hiding this comment.
This can be removed, there is already a post-proposal check that uses pcvGuardian to empty the deposits
| addresses.mainnet("PCV_DEPOSIT_MORPHO_DAI"), | ||
| addresses.mainnet("PCV_DEPOSIT_MORPHO_USDC") | ||
| addresses.mainnet("PCV_DEPOSIT_MORPHO_COMPOUND_DAI"), | ||
| addresses.mainnet("PCV_DEPOSIT_MORPHO_COMPOUND_USDC"), | ||
| addresses.mainnet("PCV_DEPOSIT_EULER_DAI"), | ||
| addresses.mainnet("PCV_DEPOSIT_EULER_USDC"), | ||
| addresses.mainnet("PCV_DEPOSIT_MORPHO_AAVE_DAI"), | ||
| addresses.mainnet("PCV_DEPOSIT_MORPHO_AAVE_DAI") |
| /// @notice withdraw all tokens from Morpho | ||
| /// non-reentrant as state changes and external calls are made | ||
| /// @param to the address PCV will be sent to | ||
| function withdrawAll(address to) external onlyPCVController globalLock(2) { |
There was a problem hiding this comment.
Add a test that calls this, it's not covered
| /// profits and or last recorded balance. | ||
| /// If a deposit records PNL, only use this | ||
| /// function in an emergency. | ||
| function withdrawERC20( |
There was a problem hiding this comment.
Add a test that calls this, it's not covered
| /// @notice this is a no-op | ||
| /// euler distributes tokens through a merkle drop, | ||
| /// no need for claim functionality | ||
| function _claim() internal pure override returns (uint256) { |
There was a problem hiding this comment.
if you don't remove it from PCVDeposit, add a (very small) test to call this, it's not covered
|
Another comment : |
Refactor PCV Deposit V2 to a cleaner architecture that speeds up integrations.
Create a Euler PCV Deposit
Create a Morpho Aave PCV Deposit
Wire both Euler and Aave PCV Deposits into the deployment script
Add integration tests for these new PCV Deposits