DAO vote 2: Incentives, deprecate PSMs, holding PCV deposits, agEUR redeem by thomas-waite · Pull Request #922 · fei-protocol/fei-protocol-core

thomas-waite · 2022-06-20T12:07:49Z

DAO vote 2: Incentives, deprecate PSMs, holding PCV deposits, agEUR redeem

Batches various cleanup and technical maintenance tasks into a second DAO vote:

Incentives cleanup/withdrawal
Introduction of Holding ERC20 PCV deposits and deprecation of remaining PSMs
Redemption of Angle and agEUR

Incentives cleanup

This proposal funds the owed outgoing incentives and then withdraws TRIBE from various areas in the system: Tribal Chief, ERC20 Dripper, Votium contracts and sends it to the Core treasury.

It also transfers the owner of the Fei Aave incentives controller from the TRIBE DAO to the Aave DAO.

Key numbers

Amount of TRIBE:

On TribalChief: 29.2M
Owed in pending rewards to LP token stakers: 565k
On ERC20 Dripper: 2.5M

Total to be recovered: 31.3M TRIBE

Implementation

Off-chain, all staking token wrapper harvests have been performed.

The final deprecation steps performed are:

Withdraw from the TribalChief all remaining TRIBE, minus the total pending rewards owed to the historical stakers of Uniswap-v2 FEI/TRIBE LP, Curve 3crv-FEI metapool LP and G-UNI DAI/FEI 0.05% fee tier. Use governorWithdrawTribe(uint256 amount)
Withdraw TRIBE from ERC20Dripper
Withdraw all TRIBE from the Votium contracts
Transfer ownership of Fei Aave incentives controller to Aave governance

proposals/description/deprecate_incentives.ts

block.txt

xklob · 2022-07-05T20:51:40Z

contracts/metagov/DelegatorPCVDeposit.sol

Confused as to the usage of "snapshot" here and previously, not sure what it means in this context

xklob · 2022-07-05T20:55:50Z

contracts/metagov/DelegatorPCVDeposit.sol

+/// This contract simply holds an ERC20 token, and delegate its voting power
+/// to an address. The ERC20 token needs to implement a delegate(address) method.
+/// @author eswak
+contract DelegatorPCVDeposit is PCVDeposit {


Until I got to the part where the metagovernance role was used, I didn't realize this contract was for that purpose and not just a standard erc20 delegation contract. Should we change the name to reflect that this contract's purpose is to be used for metagovernance?

I'm not sure what should change here, a comment or the artifact name ? We use "Delegator" in the name for all contracts that call erc20vote.delegate() and other than that it's a very simple pcv deposit

Artifact name (delegating normal votes vs using for metagovernance is the confusing part)

xklob · 2022-07-05T20:56:58Z

contracts/metagov/VlAuraDelegatorPCVDeposit.sol

+
+/// @title Vote-locked AURA PCVDeposit
+/// @author eswak
+contract VlAuraDelegatorPCVDeposit is DelegatorPCVDeposit {


Can you update the description of this contract to provide more context?

xklob · 2022-07-05T20:58:44Z

contracts/pcv/ERC20HoldingPCVDeposit.sol

+    IERC20 immutable token;
+
+    /// @notice Fei ERC20 token address
+    address private constant FEI = 0x956F47F50A910163D8BF957Cf5846D573E7f87CA;


Would prefer that we not inline this and use CoreRef instead

This was approved and deployed in this PR: #930

Would be nice to use CoreRef, but don't think it warrants making a change now and redeploying

xklob · 2022-07-05T21:00:02Z

contracts/pcv/ERC20HoldingPCVDeposit.sol

+    }
+
+    /// @notice Empty receive function to receive ETH
+    receive() external payable {}


Should add fallback as well to catch all use cases of sending eth to this contract

What use case of sending eth to this contract would a receive() function not handle, but a fallback would?

Fallback handles contract calls that don't match any function, receive() only handles transfers

contracts/pcv/angle/AngleEuroRedeemer.sol

contracts/pcv/angle/IPoolManager.sol

contracts/pcv/angle/IStableMaster.sol

xklob · 2022-07-05T21:18:36Z

contracts/test/integration/fixtures/MainnetAddresses.sol

@@ -3,13 +3,15 @@ pragma solidity ^0.8.4;

 library MainnetAddresses {


Just now seeing this library - should we ensure that this is synced with the TS mainnet addresses? (via a test or something, dunno)

I don't see the use case/additional guarantees of ensuring this is synced with the TS mainnet addresses. If there is a compelling reason, should break it out into a seperate PR

xklob · 2022-07-05T22:08:50Z

test/integration/tests/pauseState.ts

+
+  before(async function () {
+    // Setup test environment and get contracts
+    const version = 1;


What does this version number mean?

This has been part of our e2e harness since the beginning I think - I remember adding a version number back when I was contracting.

Down for a refactor of our e2e setup, but that should be broken out and likely done in Foundry

xklob · 2022-07-05T22:11:45Z

protocol-configuration/pauseState.ts

+  }
+};
+
+export type PauseState = {


Should change this to PSMPauseState, and this should be an interface instead of a type generally.

I think we can keep this name generic, there's no reason we wouldn't use this to check paused state of other contracts (in addition to PSMs)

Also think this should stay generic. It does the job that we need right now in allowing us to quickly verify pause state with a local config

xklob · 2022-07-05T22:13:00Z

protocol-configuration/pauseState.ts

@@ -0,0 +1,30 @@
+export const pauseStateConfig: PauseStateConfig = {


I think we should rethink this test. While it's good to ensure that certain contracts are either paused or not, this config and these types are too specific to PSMs.

We could name this in a generic way "state config" and use that to check various state variables. For instance we could add

lusdPSM: { paused: true, redeemPaused: true, surplusTarget: '{bammDeposit}' }

and other state checks

I think I prefer this to what we have now, though still not sure if this test/config is warranted. Discuss in dev meeting perhaps?

The initial version of this test was done to make sure we don't accidentally unpause/pause something and don't know. We'll loudly get alerted now if we do something that accidentally unpauses.

If we have need for a more thorough pause state test in the future very down for iterating and breaking out into another PR

xklob · 2022-07-05T22:14:01Z

protocol-configuration/mainnetAddresses.ts

+    address: '0x45EB1A004373b1D8457134A2C04a42d69D287724',
+    category: AddressCategory.External
+  },
+  eswak: {


I don't think we should put personal/deployer addresses in this file

it's either this or magic addresses in proposal descriptions because for instance the DAO vote has a setDelegate(address) call that needs my address as a parameter. But I'm fine with both, added this because Tom wanted to have it in the config file

Yep much prefer having any address that gets used in a DAO vote in mainnetAddresses under a human readable key, rather than having a magic address in the script

xklob · 2022-07-05T22:16:40Z

proposals/dao/tip_118.ts

+let initialCoreTribeBalance: BigNumber;
+let initialRariDelegatorBalance: BigNumber;
+
+const EXPECTED_WETH_TRANSFER = toBN('21716293570455965978548');


Please add a comment for each of these vars explaining what they mean & providing a little bit of context

Totally, good idea. Will add

proposals/description/tip_118.ts

…l/fei-protocol-core into deprecate-incentives

…otocol-core into deprecate-incentives

…l/fei-protocol-core into deprecate-incentives

…otocol-core into deprecate-incentives

eswak and others added 9 commits June 2, 2022 15:48

Add AURA locker contract

fbad5df

add initialize function

e0befcf

deploy AURA airdrop claimer contract

6f56d32

Merge remote-tracking branch 'origin/develop' into feat/aura-locker

b680a68

feat: initial commit

83ec87b

feat: update standard fip template

426df3a

feat: withdraw TRIBE from various and send to core

17f9e5f

refactor: use the DAO to pull from TribalChief

b29336f

feat: harvest staking token wrappers

61e3b82

thomas-waite added the devops/process label Jun 20, 2022

thomas-waite self-assigned this Jun 20, 2022

thomas-waite added 4 commits June 20, 2022 14:14

refactor: harvest all STWs and withdraw more from votium

89c7af1

feat: pull from other votium, add validation

8914779

feat: change Aave incencentives controller proxy admin

1048638

Merge branch 'develop' into deprecate-incentives

a463428

thomas-waite changed the title ~~[WIP] Deprecate incentives~~ Deprecate incentives Jun 20, 2022

thomas-waite marked this pull request as ready for review June 20, 2022 14:06

thomas-waite requested a review from a team as a code owner June 20, 2022 14:06

thomas-waite added 2 commits June 20, 2022 17:04

feat: extract excess Rari TRIBE rewards

1172404

feat: revoke unneeded roles and deprecate contracts

576ad46

thomas-waite commented Jun 20, 2022

View reviewed changes

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

thomas-waite commented Jun 20, 2022

View reviewed changes

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

thomas-waite commented Jun 20, 2022

View reviewed changes

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

thomas-waite commented Jun 20, 2022

View reviewed changes

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

thomas-waite force-pushed the deprecate-incentives branch from 8afa448 to 576ad46 Compare June 20, 2022 16:25

thomas-waite added 5 commits June 20, 2022 17:31

refactor: minor touchups

2a7e69f

refactor: check final tribe balance

99e8fc9

refactor: keep TRIBE in timelock, already harvest

8b9c21f

refactor: perform on-chain harvests, update

d103dbe

docs: update proposal description

00fbf62

thomas-waite and others added 4 commits July 5, 2022 17:04

test: account for Fei deposits in equity check

4bd52bf

refactor: update CR config and e2e tests

9b576bd

docs: update fei burn comment

b72a2fc

Merge branch 'develop' into deprecate-incentives

581d9f1

xklob reviewed Jul 5, 2022

View reviewed changes

proposals/description/tip_118.ts Show resolved Hide resolved

thomas-waite and others added 18 commits July 6, 2022 13:46

refactor: increase unit test runners

8111c05

Merge branch 'deprecate-incentives' of https://github.com/fei-protoco…

32d7e54

…l/fei-protocol-core into deprecate-incentives

deps: update block num

029ef38

docs: add comments to constants in DAO vote

5f91b60

refactor: loosen DAI agEUR slippage check

0bdc2f7

docs: add forum post and code to proposal

32e805d

refactor: reduce check to 8M, order of magnitude

a3313c4

test: trigger CI run and log address

cfffff7

refactor: remove .only

6100eae

klob's tip-118 review

5f29a06

Merge branch 'deprecate-incentives' of github.com:fei-protocol/fei-pr…

39f76d1

…otocol-core into deprecate-incentives

docs: cleanup DAO proposal

f66b0f0

Merge branch 'deprecate-incentives' of https://github.com/fei-protoco…

7e7e113

…l/fei-protocol-core into deprecate-incentives

Update Angle artifact names in mainnetAddresses

0e1fca5

Merge branch 'deprecate-incentives' of github.com:fei-protocol/fei-pr…

d96810b

…otocol-core into deprecate-incentives

test: account for any eth already at test address

8a1e55a

refactor: explicitly determine overloaded propose method

ba920bf

test: sort out initial eth balance

847de09

Joeysantoro approved these changes Jul 6, 2022

View reviewed changes

thomas-waite merged commit 4a894fa into develop Jul 6, 2022

thomas-waite deleted the deprecate-incentives branch July 6, 2022 21:01

eswak mentioned this pull request Jul 7, 2022

Deprecate ethPSMRouter (the only PSMRouter) #961

Merged

		@@ -3,13 +3,15 @@ pragma solidity ^0.8.4;

		library MainnetAddresses {

		@@ -0,0 +1,30 @@
		export const pauseStateConfig: PauseStateConfig = {

Conversation

thomas-waite commented Jun 20, 2022 • edited by xklob Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DAO vote 2: Incentives, deprecate PSMs, holding PCV deposits, agEUR redeem

Incentives cleanup

Key numbers

Implementation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

xklob Jul 6, 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!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

xklob Jul 6, 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!

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!

Reviewers

Assignees

Labels

thomas-waite commented Jun 20, 2022 •

edited by xklob

Loading

xklob Jul 6, 2022 •

edited

Loading

xklob Jul 6, 2022 •

edited

Loading