FIP-82b: Configure roles so TribalCouncil can operate#678

Merged

thomas-waite merged 59 commits intodevelopfrom

feat-role-transfer

May 4, 2022

Contributor

thomas-waite commented Apr 10, 2022 •

edited

Loading

Summary

Configures roles and contract admins throughout the system so that the TribalCouncil can operate the protocol on an operational basis. Specifically:

Transfers admin of all non-major TribalRoles to the ROLE_ADMIN. ROLE_ADMIN is held by the TribalCouncil, which will allow it to grant and revoke all non-major roles
According to an initial spec, creates new roles to act as the contract admin of certain contracts. Sets the admin of these new roles to be the ROLE_ADMIN calls the individual contracts to set the CONTRACT_ADMIN_ROLE
Grants newly created operational roles to the TribalCouncil timelock
If a contract admin was changed, then it grants the new contract admin role to whichever contract used to be granted the previous contract admin role. This prevents any bricking of the system, any previous admin contracts will still have admin functionality

Changed admin functionality

FuseGuardian

Prev: TRIBAL_CHIEF_ADMIN_ROLE
New admin role: FUSE_ADMIN
Granted new to: optimisticTimelock,tribalChiefSyncV2, 'tribalCouncilTimelock`

optimisticMinter

Prev: GOVERN_ROLE
New: FEI_MINT_ADMIN
Granted new to: 'feiDAOTimelock', 'tribalCouncilTimelock`

pcvEquityMinter

Prev: GOVERN_ROLE
New: FEI_MINT_ADMIN
Granted new to: 'feiDAOTimelock', 'tribalCouncilTimelock'

ethLidoPCVDeposit

Implementation on chain does not use a CoreRef implementation that has a setContractAdminRole() function. Removed.

indexDelegator

Prev: METAGOVERNANCE_VOTE_ADMIN
New: METAGOVERNANCE_VOTE_ADMIN (no change)
Granted new to: feiDAOTimelock', 'opsOptimisticTimelock', 'tribalCouncilTimelock

ethTokemakPCVDeposit

Unknown admin role: 0x6c9ecf07a5886fd74a8d32f4d3c317a7d5e5b5c7a073a3ab06c217e9ce5288e3
Prev: TOKEMAK_DEPOSIT_ADMIN_ROLE
New: TOKEMAK_DEPOSIT_ADMIN_ROLE (no change)
Granted to: feiDAOTimelock, optimisticTimelock, tribalCouncilTimelock

uniswapPCVDeposit

Prev: GOVERN_ROLE
New: PCV_MINOR_PARAM_ROLE
Granted new to: 'feiDAOTimelock', optimisticTimelock,tribalCouncilTimelock

daiPSMFeiSkimmer

Prev: GOVERN_ROLE
New: PCV_MINOR_PARAM_ROLE
Granted new to: 'feiDAOTimelock', optimisticTimelock,tribalCouncilTimelock

lusdPSMFeiSkimmer

Prev: GOVERN_ROLE
New: PCV_MINOR_PARAM_ROLE
Granted new to: 'feiDAOTimelock', optimisticTimelock,tribalCouncilTimelock

ethPSMFeiSkimmer

Prev: GOVERN_ROLE
New: PCV_MINOR_PARAM_ROLE
Granted new to: 'feiDAOTimelock', optimisticTimelock,tribalCouncilTimelock

aaveEthPCVDripController

Prev: GOVERN_ROLE
New: PCV_MINOR_PARAM_ROLE
Granted new to: 'feiDAOTimelock', optimisticTimelock,tribalCouncilTimelock

daiPCVDripController

Prev: GOVERN_ROLE
New: PCV_MINOR_PARAM_ROLE
Granted new to: 'feiDAOTimelock', optimisticTimelock,tribalCouncilTimelock

lusdPCVDripController

Prev: GOVERN_ROLE
New: PCV_MINOR_PARAM_ROLE
Granted new to: 'feiDAOTimelock', optimisticTimelock,tribalCouncilTimelock

compoundEthPCVDripController

Prev: GOVERN_ROLE
New: PCV_MINOR_PARAM_ROLE
Granted new to: 'feiDAOTimelock', optimisticTimelock,tribalCouncilTimelock

thomas-waite added 3 commits

April 10, 2022 22:36


          feat: migrate all non-major TribeRoles

06dee76


          feat: create new TribeRoles


          feat: migrate contract admin roles

739183b

thomas-waite self-assigned this

thomas-waite requested a review from a team as a code owner

April 10, 2022 23:09

thomas-waite changed the base branch from feat-governance-upgrade to gov-todos

April 10, 2022 23:09

thomas-waite added 2 commits

April 11, 2022 00:12


          feat: validate TribalCouncil granted roles

53032c3


          test: increase max proposals

0ca7810

thomas-waite force-pushed the feat-role-transfer branch from 0886ef4 to 0ca7810 Compare

April 10, 2022 23:29

thomas-waite mentioned this pull request

Governance TODOs #674

Merged

thomas-waite added 3 commits

April 11, 2022 17:28


          Merge branch 'gov-todos' into feat-role-transfer

bff1e8b


          fix: PR feedback

5cd91a1


          Merge branch 'gov-todos' into feat-role-transfer

12f01df

thomas-waite force-pushed the feat-role-transfer branch 2 times, most recently from ad54ad3 to e12ac60 Compare

April 11, 2022 20:54

thomas-waite commented

View reviewed changes

contracts/core/TribeRoles.sol Outdated Show resolved Hide resolved

thomas-waite commented

View reviewed changes

proposals/dao/fip_82b.ts Show resolved Hide resolved

thomas-waite commented

View reviewed changes

proposals/dao/fip_82b.ts Show resolved Hide resolved

thomas-waite commented

View reviewed changes

proposals/dao/fip_82b.ts Outdated Show resolved Hide resolved

thomas-waite commented

View reviewed changes

proposals/dao/fip_82b.ts Outdated Show resolved Hide resolved

thomas-waite commented

View reviewed changes

proposals/dao/fip_82b.ts Show resolved Hide resolved

thomas-waite commented

View reviewed changes

proposals/dao/fip_82b.ts Outdated

+                expect(await contracts.ethTokemakPCVDeposit.CONTRACT_ADMIN_ROLE()).to.be.equal(ethers.utils.id('PCV_ADMIN'));
+                expect(await contracts.uniswapPCVDeposit.CONTRACT_ADMIN_ROLE()).to.be.equal(ethers.utils.id('PCV_ADMIN'));
+                expect(await contracts.daiPSMFeiSkimmer.CONTRACT_ADMIN_ROLE()).to.be.equal(ethers.utils.id('PCV_ADMIN'));

Contributor Author

thomas-waite Apr 11, 2022 •

edited

Loading

Note, @Joeysantoro we're revisiting PCV_MINOR_PARAM_ROLE, possibly to more closely scope it's powers

thomas-waite commented

View reviewed changes

proposals/dao/fip_82b.ts Show resolved Hide resolved

thomas-waite commented

View reviewed changes

proposals/dao/fip_82b.ts Show resolved Hide resolved


          refactor: break out into seperate DAO vote

2c0cc53

thomas-waite force-pushed the feat-role-transfer branch from e12ac60 to 2c0cc53 Compare

April 12, 2022 00:14

thomas-waite changed the title ~~Configure roles so TribalCouncil can operate~~ FIP-82b: Configure roles so TribalCouncil can operate


          feat: transfer all remaining non-major roles

50cc766

thomas-waite force-pushed the feat-role-transfer branch from a9dead0 to 50cc766 Compare

April 12, 2022 01:16


          this works

658697f


          Merge pull request #772 from fei-protocol/split-pcv-guard-role

8da609d

Add PCV_SAFE_MOVER_ROLE

thomas-waite commented

View reviewed changes

contracts/core/TribeRoles.sol Show resolved Hide resolved

Contributor

Joeysantoro commented May 4, 2022

Made this jist clarifying the PR description accuracy https://gist.github.com/Joeysantoro/21d92cbea44bde4476814952424427fb/revisions

Contributor

Joeysantoro commented May 4, 2022

Before we can submit I'd like to see these new role changes reflected in the governance and roles doc https://fei-protocol.github.io/docs/docs/protocol/Governance/detailedAccessControl#fuse_admin

Joeysantoro reviewed

View reviewed changes

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

Joeysantoro reviewed

View reviewed changes

proposals/description/fip_82b.ts

+                    values: '0',
+                    method: 'createRole(bytes32,bytes32)',
+                    arguments: [
+                      '0xf0b50f04623eeaacfa1f202e062a3001c925a35c6b75d6903e67b43f44bbf152', // PARAMETER_ADMIN

Contributor

Joeysantoro May 4, 2022 •

edited

Loading

PARAMETER_ADMIN doesn't appear anywhere in the protocol or the permissions test, and I don't think we have plans to use it. I prefer removing it from TribeRoles and this script

Contributor Author

thomas-waite May 4, 2022 •

edited

Loading

It's used in the NonCustodialPSM, but hasn't been created or granted out yet. We should leave as it's in an access modifier on NonCustodialPSM - have added to permissions.ts

Contributor

Joeysantoro May 4, 2022

imo the roles should be consolidated/clarified. There is a PSM_ADMIN_ROLE and a MINOR_PARAMETER_ADMIN. At least one of those should replace this one or vice versa

Contributor Author

thomas-waite May 4, 2022

As discussed, leaving for now

Joeysantoro reviewed

View reviewed changes

proposals/description/fip_82b.ts Show resolved Hide resolved

Joeysantoro reviewed

View reviewed changes

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

Joeysantoro reviewed

View reviewed changes

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

Joeysantoro reviewed

View reviewed changes

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

Joeysantoro reviewed

View reviewed changes

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

thomas-waite added 8 commits

May 4, 2022 13:25


          refactor: PR feedback, remove roles

dfa5339


          docs: update FIP description

4c459ba


          Merge branch 'feat-role-transfer' of https://github.com/fei-protocol/…

30b5eb4

…fei-protocol-core into feat-role-transfer


          Revert "Merge branch 'feat-role-transfer' of https://github.com/fei-p…

9ae6414

…rotocol/fei-protocol-core into feat-role-transfer"

This reverts commit 30b5eb4, reversing
changes made to 4c459ba.


          Merge branch 'feat-role-transfer' of https://github.com/fei-protocol/…

30e9dd6

…fei-protocol-core into feat-role-transfer


          fix: correct broken merge

cbebe54


          feat: add PARAMETER_ADMIN to permissions

f6c79ee


          refactor: change tokemak and index delegator admins

ee41371

thomas-waite force-pushed the feat-role-transfer branch from 26780f1 to ee41371 Compare

May 4, 2022 16:22

Contributor Author

thomas-waite commented May 4, 2022

Made this jist clarifying the PR description accuracy https://gist.github.com/Joeysantoro/21d92cbea44bde4476814952424427fb/revisions

Updated PR description with this and made the two role changes in there

Contributor Author

thomas-waite commented May 4, 2022

Before we can submit I'd like to see these new role changes reflected in the governance and roles doc https://fei-protocol.github.io/docs/docs/protocol/Governance/detailedAccessControl#fuse_admin

Updated


          feat: take admin of two more roles

48acdf9

Joeysantoro previously approved these changes

View reviewed changes


          test: make dependency test pass

ed4d535

thomas-waite dismissed Joeysantoro’s stale review via

ed4d535

May 4, 2022 20:02


          refactor: remove redundant role

1163ccc

Joeysantoro approved these changes

View reviewed changes

thomas-waite merged commit 0c9a37e into develop

thomas-waite deleted the feat-role-transfer branch

May 4, 2022 21:07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet