Conversation
xklob
left a comment
xklob
left a comment
There was a problem hiding this comment.
This is awesome, some comments.
I'd also like to see unit tests for BPTLens.sol (and also maybe go over it together over zoom?)
|
|
||
| chai.use(CBN(ethers.BigNumber)); | ||
|
|
||
| const CHAINLINK_LUSD = '0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0'; |
There was a problem hiding this comment.
Why not add these to mainnet addresses instead?
There was a problem hiding this comment.
these seemed transient. Because they are external I'd rather just keep them out. That file is getting unwieldy
| // 2. Bal-Eth oracle | ||
| const chainlinkBALEthOracle = await chainlinkFactory.deploy(core, CHAINLINK_BAL); | ||
|
|
||
| await chainlinkBALEthOracle.deployed(); |
There was a problem hiding this comment.
I believe it's actually
await chainlinkBALETHOracle.deployTransaction.wait()
There was a problem hiding this comment.
Looks like the same thing https://github.com/ethers-io/ethers.js/blob/master/packages/contracts/src.ts/index.ts#L815-L828
There was a problem hiding this comment.
I think we should also add these :
Oracle price for stkAAVE tokenERC20 Lens for stkAAVE on the Aave ETH depositERC20 Lens for stkAAVE on the Aave RAI deposit- Oracle price for COMP token
- ERC20 Lens for COMP on the Compound ETH deposit
- ERC20 Lens for COMP on the Compound DAI deposit
- Oracle price for TOKE token
- Add the TOKE Tokemak deposit
I'd also update the Angle proposal to add ANGLE price feed, and a lens on the deposit, but I don't think they have a good price feed yet ? I'll ask them on Telegram.
edit: correction, the Aave reward claims are not permissionless, we'll need to write a new contract for that. We have ~630 stkAAVE rewards to claim (~160k$) for the ETH deposit, but we can't claim them permissionlessly, we need to write a new "reward claimer" contract, ask the Aave team to authorize it as a claimer for our PCVDeposits, and then we'll be able to claim. We also have ~63 stkAAVE from the RAI deposit. Claim happens on 0xd784927Ff2f95ba542BfC824c8a8a98F3495f6b5. Maybe we wait for a more in-depth rework of the rewards claimer / moving architecture, before adding these...
I don't think its worth adding these until they make up a larger percentage of PCV and we have better infrastructure for deploying them |
|
check e2e tests before merge & we are go for launch ? |
eswak
left a comment
eswak
left a comment
There was a problem hiding this comment.
Code lgtm, one last thing : the feiBuybackLens at 0x107460564896377BA6CdcC7516c7eAb65E32E360 is not verified
Also, can you break me down what the 61.5M FEI from the static PCV deposit are for ? And do you confirm the 2M$ are for the INDEX holdings ?
Eventually you can add 500k for FEI Idle AA tranche and 2M FEI Visor, or I'll queue an OA action for it after I'm done writing the full breakdown to make sure we still have not forgot something else
| import dependencies from '@addresses/dependencies'; | ||
|
|
||
| describe('e2e-dependencies', function () { | ||
| describe.skip('e2e-dependencies', function () { |
There was a problem hiding this comment.
did you intend to commit this ?
3 participants
Updates the CollateralizationOracle values to have the most up to date PCV numbers:
Add -
Remove the 2 duplicate Fuse deposits and old staticPcvDepositWrapper
Adds BPTLens to be able to calculate the resistant balances of the LBP tokens. Uses gas efficient calculations by deriving optimized versions of the ideal reserves formula
Replaces existing StaticPCVDepositWrapper with a new one that allows for incrementing and decrementing with new values: