TIP-111: Increase stable backing to 90-100% by eswak · Pull Request #897 · fei-protocol/fei-protocol-core

eswak · 2022-06-11T16:35:09Z

PR Type: Feature

DAO action :

Add an oracle on the stETH PCVDeposit, and migrate stETH from the old contract to a new contract
Unstake all B-70WETH-30FEI from Balancer gauge, remove liquidity, burn FEI, and send WETH to PSM
FIP_104b actions (Prepare LBP withdraw #821)

PR Checklist - Feature (Proposal)

thomas-waite

Know this is WIP, few clarifying questions

contracts/pcv/lido/EthLidoPCVDeposit.sol

proposals/dao/migrate_lido_steth.ts

thomas-waite

This looks good to me. Small comments. When we merge this, I'll consolidate into one DAO vote to also pull the DAI out of the DPI LBP

contracts/oracle/CompositeOracle.sol

proposals/dao/tip_111.ts

protocol-configuration/mainnetAddresses.ts

proposals/description/tip_111.ts

proposals/dao/tip_111.ts

…oracle

thomas-waite

Looks good to me

proposals/dao/tip_111.ts

thomas-waite · 2022-06-13T20:09:11Z

proposals/description/tip_111.ts

+        '10000' // basisPoints, 100%
+      ],
+      description:
+        'Withdraw all DPI from the Compound DAI PCV deposit (~$200k) to the TribalCouncil multisig, where it will be liquidated'


This is what I had originally put together - send the leftover DPI to the TC safe where it can be liquidated on a DEX aggregator. Flagging here for others, it's a small amount, appropriate imo

test/unit/pcv/lido/EthLidoPCVDeposit.test.ts

xklob · 2022-06-13T19:55:01Z

protocol-configuration/mainnetAddresses.ts

+  convexPoolPCVDeposit: {
+    artifactName: 'ERC20CompoundPCVDeposit',
+    address: '0x525eA5983A2e02abA8aA0BE7D15Cd73150812379',
+    category: AddressCategory.PCV_V1


Is this actually PCV_V1?

fun fact but this PCV deposit is actually using the PCV_V2 interface, idk why but a wrapper has been deployed, and it's the wrapper that is in the CR oracle (even though there was no need for wrapping)

If we flag this as PCV, the e2e automatically expect it to be configured in the CR oracle, which is not the case

I added it because it was not in our mainnetAddresses file & that's a problem because it's holding protocol funds (only the wrapper was in mainnetAddresses)

That's so weird

xklob · 2022-06-13T20:20:08Z

test/unit/pcv/lido/EthLidoPCVDeposit.test.ts

    this.fei = await ethers.getContractAt('Fei', await this.core.fei());
    this.steth = await (await ethers.getContractFactory('MockStEthToken')).deploy();
    this.stableswap = await (await ethers.getContractFactory('MockStEthStableSwap')).deploy(this.steth.address);
+    this.oracle = await (await ethers.getContractFactory('MockOracle')).deploy(1);


We should remove references to this in this file - I know most of them were already here but this is bad practice as this bypasses type checking since these vars are created at runtime.

xklob · 2022-06-13T20:22:36Z

test/unit/pcv/lido/EthLidoPCVDeposit.test.ts

      await ethers.getContractFactory('EthLidoPCVDeposit')
    ).deploy(
      this.core.address,
+      {


Getting some errors on typing for the oracle params, are you sure this is correct? My types show that the second param here should be a string.

you may need to recompile, I had this problem while developing, the old types were used and I got a lot of warnings in my editor. I have added a structure for oracle params in the constructor, but the old 2nd parameter was a string (now the 3rd parameter is a string, and 2nd is a structure)

one more reason why I don't like that we add a compilation layer on top of js btw

proposals/dao/tip_111.ts

xklob · 2022-06-13T20:35:24Z

contracts/oracle/CompositeOracle.sol

Since this is functionally the same as doInvert, should we use that verbage so that it's the same across oracles?

I'd add this naming has confusing boolean interpretations. It is not multiply || divide

should we just name it multiply ?

doInvert to me means you're directly inverting something - 1 / quantity. Not sure of the smoothest syntax. Maybe two boolean parameters:

bool multiply bool divide require(multiply != true && divide != true, "Can not set multiply and divide to true")

we settled for invertOracleB in dev chat, used that

proposals/description/tip_111.ts

Joeysantoro · 2022-06-13T23:32:47Z

proposals/dao/tip_111.ts

+  expect(stethBalanceBefore).to.be.at.most(51000);
+
+  // --------------------------------------------------------------
+  // FIP_104b validation


nit: FIP_104 incorrect

these are actually the steps from Tom's fip_104, that have been combined with tip_111 to not need a 2nd dao vote

I called it FIP_104b to indicate it was the second part/cleanup (FIP_104 started the LBP, transferred CREAM and did various technical maintenance tasks)

Joeysantoro · 2022-06-14T12:47:06Z

proposals/description/tip_111.ts

+      values: '0',
+      method: 'deposit()',
+      arguments: () => [],
+      description: 'Deposit all WETH in Aave'


we may not want to do this, Aave could have solvency issues with stETH leverage

I'm adding a call to move WETH to PSM (we can pause PSM with guardian prior to execution)

This reverts commit a4ddd8e.

thomas-waite

Looks good to me

eswak added 2 commits June 11, 2022 18:30

(wip) steth new contract + proposal

5411adb

fix BalancerGaugeStaker e2e test

59bf12f

thomas-waite reviewed Jun 13, 2022

View reviewed changes

update tip-111 to use composite oracle

101e6d3

eswak changed the title ~~(wip) steth new contract + proposal~~ TIP-111: Increase stable backing to 90-100% Jun 13, 2022

thomas-waite reviewed Jun 13, 2022

View reviewed changes

eswak added 5 commits June 13, 2022 18:14

fix unit tests & protocol config

12e318d

TIP-111 Tom's review

1a603b7

fix default parameter value in CompositeOracle unit tests

9fa68f7

Merge remote-tracking branch 'origin/develop' into feat/lido-deposit-…

10c9505

…oracle

merge fip_104b into tip_111

0850250

eswak marked this pull request as ready for review June 13, 2022 19:32

eswak requested a review from a team as a code owner June 13, 2022 19:32

thomas-waite reviewed Jun 13, 2022

View reviewed changes

xklob suggested changes Jun 13, 2022

View reviewed changes

Joeysantoro reviewed Jun 13, 2022

View reviewed changes

proposals/description/tip_111.ts Outdated Show resolved Hide resolved

Joeysantoro reviewed Jun 13, 2022

View reviewed changes

Joeysantoro reviewed Jun 14, 2022

View reviewed changes

eswak added 10 commits June 14, 2022 16:58

type all the things (except one)

ed76159

remove proposals that already executed from proposalsConfig

a4ddd8e

update block

d07f0b7

use ethers.utils instead of bytes32 for role

4693a88

Revert "remove proposals that already executed from proposalsConfig"

f0ddb53

This reverts commit a4ddd8e.

fix conflict merge err

e216ca6

fix tests

2c87440

fix tests (2)

62d2317

fix tests (3)

ebf6d42

type pcvStats

ab391e8

thomas-waite previously approved these changes Jun 15, 2022

View reviewed changes

fix ts errors in deploy script

bf55c6a

deploy and update fork block

fa9d5ae

eswak dismissed thomas-waite’s stale review via fa9d5ae June 15, 2022 12:56

Joeysantoro approved these changes Jun 15, 2022

View reviewed changes

xklob approved these changes Jun 15, 2022

View reviewed changes

Merge branch 'develop' into feat/lido-deposit-oracle

ff120b6

thomas-waite merged commit f155def into develop Jun 15, 2022

thomas-waite deleted the feat/lido-deposit-oracle branch June 15, 2022 20:02

Conversation

eswak commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Type: Feature

PR Checklist - Feature (Proposal)

Uh oh!

thomas-waite left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thomas-waite left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thomas-waite left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Joeysantoro Jun 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomas-waite left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

eswak commented Jun 11, 2022 •

edited

Loading

Joeysantoro Jun 13, 2022 •

edited

Loading