-
Notifications
You must be signed in to change notification settings - Fork 99
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
Fei/Volt Swap #719
Fei/Volt Swap #719
Conversation
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.
Looks good to me, small points
voltFusePCVDeposit: { | ||
artifactName: 'ERC20CompoundPCVDeposit', | ||
address: '0xFeBDf448C8484834bb399d930d7E1bdC773E23bA', | ||
category: AddressCategory.PCV | ||
}, |
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.
Tbh I don't love the intermingling here. Lets think about having a parallel mainnetAddresses configuration in volt or even rewriting the proposal flow in forge
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 already did a simulation in the volt repo with forge, but it's not the same thing as writing out the calldata steps here. https://github.com/volt-protocol/volt-protocol-core/pull/70/files
export const deploy: DeployUpgradeFunc = async (deployAddress, addresses, logging = false) => { | ||
const TOKEN_DEPOSIT = process.env.TOKEN_DEPOSIT; | ||
const TOKEN = process.env.TOKEN; | ||
const IS_PROTOCOL_FEI_DEPOSIT = |
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 don't like adding new env vars that need to be set for deployment scripts that will be re-used; this is fine for now but in the future I want to move to hardhat tasks so that we can pass in these parameters via CLI.
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.
Thanks for adding the TurboPCVDeposit to the Guardian for me
DAO Steps:
to the VOLT PCV Deposit so that the funds do not get invested into fuse
will be reflected in the PCV