Skip to content
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

Withdraw 30M LP tokens from Convex and 10M Fei from D3 Pool #791

Merged
merged 20 commits into from
May 17, 2022

Conversation

thomas-waite
Copy link
Contributor

@thomas-waite thomas-waite commented May 10, 2022

Summary

Withdraws 30M Fei from the Curve D3 pool.

Uses the PCVGuardian to first pull the LP tokens from the Convex PCVDeposit and send them to the Curve PCVDeposit. Then withdraws 10M Fei from the Curve PCVDeposit and sends it to the DAI PSM. Two further transactions will be made in the future to withdraw additional Fei. Makes use of the TribalCouncil to do this.

A Fei Skimmer is also deployed for the DAI PSM to enable Fei to be skimmed off the DAI PSM. A DAO vote is required to do this, as the PCV_CONTROLLER_ROLE needs granting.

@thomas-waite thomas-waite requested a review from a team as a code owner May 10, 2022 15:35
@thomas-waite thomas-waite self-assigned this May 10, 2022
@thomas-waite thomas-waite changed the title [WIP] Withdraw 30M from D3 Pool Withdraw 30M from D3 Pool May 10, 2022
@thomas-waite thomas-waite changed the title Withdraw 30M from D3 Pool Withdraw 30M LP tokens from Convex and 10M Fei from D3 Pool May 10, 2022
kryptoklob
kryptoklob previously approved these changes May 10, 2022

/// @notice Set the target to skim from. Only Governor or Admin
/// @param newSource the new source to skim from
function setSource(address newSource) external onlyGovernorOrAdmin {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you're going to use the onlyGovernorOrAdmin modifier, you have to set the admin of this contract in the constructor

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's set by default in CoreRef, which this contract inherits from, to GOVERNOR. That's a bit redundant at the moment as the GOVERNOR can call this method anyway, but the contract admin could be changed in the future to something else.

Should stay as is imo

Copy link
Contributor Author

@thomas-waite thomas-waite May 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have thought about this some more. In the recent governance upgrade, we changed the admin of the feiSkimmer contracts to the PCV_MINOR_PARAM_ROLE. That role is intended to only manage minor PCV related parameters and should not have the scope to change the source in my opinion (as it would get in this change).

Going to change setSource to be onlyGovernor and then setContractAdmin internally to PCV_MINOR_PARAM_ROLE to avoid having to do later in a DAO vote.

Will then redeploy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, that makes sense!

ElliotFriedman
ElliotFriedman previously approved these changes May 13, 2022
kryptoklob
kryptoklob previously approved these changes May 16, 2022
@thomas-waite thomas-waite merged commit 465f710 into develop May 17, 2022
@thomas-waite thomas-waite deleted the withdraw-from-d3pool branch May 17, 2022 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants