Skip to content

Commit 8da609d

Browse files
thomas-waitethomas-waite
authored
Merge pull request #772 from fei-protocol/split-pcv-guard-role
Add PCV_SAFE_MOVER_ROLE
2 parents 2f98c71 + da1a2e3 commit 8da609d

File tree

9 files changed

+201
-71
lines changed

9 files changed

+201
-71
lines changed

‎contracts/core/TribeRoles.sol‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,6 @@ library TribeRoles {
101101
/// @notice capable of changing PCV Deposit and Global Rate Limited Minter in the PSM
102102
bytes32 internal constant PSM_ADMIN_ROLE = keccak256("PSM_ADMIN_ROLE");
103103

104-
/// @notice capable of moving PCV on the PCVGuardian
105-
bytes32 internal constant PCV_MOVER = keccak256("PCV_MOVER");
104+
/// @notice capable of moving PCV between safe addresses on the PCVGuardian
105+
bytes32 internal constant PCV_SAFE_MOVER_ROLE = keccak256("PCV_SAFE_MOVER_ROLE");
106106
}

‎contracts/pcv/PCVGuardian.sol‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import "../refs/CoreRef.sol";
66
import "./IPCVGuardian.sol";
77
import "./IPCVDeposit.sol";
88
import "../libs/CoreRefPauseableLib.sol";
9+
import {TribeRoles} from "../core/TribeRoles.sol";
910

1011
contract PCVGuardian is IPCVGuardian, CoreRef {
1112
using CoreRefPauseableLib for address;
@@ -15,8 +16,6 @@ contract PCVGuardian is IPCVGuardian, CoreRef {
1516
EnumerableSet.AddressSet private safeAddresses;
1617

1718
constructor(address _core, address[] memory _safeAddresses) CoreRef(_core) {
18-
_setContractAdminRole(keccak256("PCV_GUARDIAN_ADMIN_ROLE"));
19-
2019
for (uint256 i = 0; i < _safeAddresses.length; i++) {
2120
_setSafeAddress(_safeAddresses[i]);
2221
}
@@ -35,34 +34,50 @@ contract PCVGuardian is IPCVGuardian, CoreRef {
3534
return safeAddresses.values();
3635
}
3736

38-
// ---------- Governor-or-Admin-Only State-Changing API ----------
37+
// ---------- GOVERNOR-or-PCV_GUARDIAN_ADMIN-Only State-Changing API ----------
3938

4039
/// @notice governor-only method to set an address as "safe" to withdraw funds to
4140
/// @param pcvDeposit the address to set as safe
42-
function setSafeAddress(address pcvDeposit) external override onlyGovernorOrAdmin {
41+
function setSafeAddress(address pcvDeposit)
42+
external
43+
override
44+
hasAnyOfTwoRoles(TribeRoles.GOVERNOR, TribeRoles.PCV_GUARDIAN_ADMIN)
45+
{
4346
_setSafeAddress(pcvDeposit);
4447
}
4548

4649
/// @notice batch version of setSafeAddress
4750
/// @param _safeAddresses the addresses to set as safe, as calldata
48-
function setSafeAddresses(address[] calldata _safeAddresses) external override onlyGovernorOrAdmin {
51+
function setSafeAddresses(address[] calldata _safeAddresses)
52+
external
53+
override
54+
hasAnyOfTwoRoles(TribeRoles.GOVERNOR, TribeRoles.PCV_GUARDIAN_ADMIN)
55+
{
4956
require(_safeAddresses.length != 0, "empty");
5057
for (uint256 i = 0; i < _safeAddresses.length; i++) {
5158
_setSafeAddress(_safeAddresses[i]);
5259
}
5360
}
5461

55-
// ---------- Governor-or-Admin-Or-Guardian-Only State-Changing API ----------
62+
// ---------- GOVERNOR-or-PCV_GUARDIAN_ADMIN-Or-GUARDIAN-Only State-Changing API ----------
5663

5764
/// @notice governor-or-guardian-only method to un-set an address as "safe" to withdraw funds to
5865
/// @param pcvDeposit the address to un-set as safe
59-
function unsetSafeAddress(address pcvDeposit) external override isGovernorOrGuardianOrAdmin {
66+
function unsetSafeAddress(address pcvDeposit)
67+
external
68+
override
69+
hasAnyOfThreeRoles(TribeRoles.GOVERNOR, TribeRoles.GUARDIAN, TribeRoles.PCV_GUARDIAN_ADMIN)
70+
{
6071
_unsetSafeAddress(pcvDeposit);
6172
}
6273

6374
/// @notice batch version of unsetSafeAddresses
6475
/// @param _safeAddresses the addresses to un-set as safe
65-
function unsetSafeAddresses(address[] calldata _safeAddresses) external override isGovernorOrGuardianOrAdmin {
76+
function unsetSafeAddresses(address[] calldata _safeAddresses)
77+
external
78+
override
79+
hasAnyOfThreeRoles(TribeRoles.GOVERNOR, TribeRoles.GUARDIAN, TribeRoles.PCV_GUARDIAN_ADMIN)
80+
{
6681
require(_safeAddresses.length != 0, "empty");
6782
for (uint256 i = 0; i < _safeAddresses.length; i++) {
6883
_unsetSafeAddress(_safeAddresses[i]);
@@ -81,7 +96,7 @@ contract PCVGuardian is IPCVGuardian, CoreRef {
8196
uint256 amount,
8297
bool pauseAfter,
8398
bool depositAfter
84-
) external override isGovernorOrGuardianOrAdmin {
99+
) external override hasAnyOfThreeRoles(TribeRoles.GOVERNOR, TribeRoles.PCV_SAFE_MOVER_ROLE, TribeRoles.GUARDIAN) {
85100
require(isSafeAddress(safeAddress), "Provided address is not a safe address!");
86101

87102
pcvDeposit._ensureUnpaused();
@@ -111,7 +126,7 @@ contract PCVGuardian is IPCVGuardian, CoreRef {
111126
uint256 amount,
112127
bool pauseAfter,
113128
bool depositAfter
114-
) external override isGovernorOrGuardianOrAdmin {
129+
) external override hasAnyOfThreeRoles(TribeRoles.GOVERNOR, TribeRoles.PCV_SAFE_MOVER_ROLE, TribeRoles.GUARDIAN) {
115130
require(isSafeAddress(safeAddress), "Provided address is not a safe address!");
116131

117132
pcvDeposit._ensureUnpaused();
@@ -142,7 +157,7 @@ contract PCVGuardian is IPCVGuardian, CoreRef {
142157
uint256 amount,
143158
bool pauseAfter,
144159
bool depositAfter
145-
) external override isGovernorOrGuardianOrAdmin {
160+
) external override hasAnyOfThreeRoles(TribeRoles.GOVERNOR, TribeRoles.PCV_SAFE_MOVER_ROLE, TribeRoles.GUARDIAN) {
146161
require(isSafeAddress(safeAddress), "Provided address is not a safe address!");
147162

148163
pcvDeposit._ensureUnpaused();

‎proposals/dao/fip_82.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,14 @@ import {
77
TeardownUpgradeFunc,
88
ValidateUpgradeFunc
99
} from '@custom-types/types';
10-
import { getImpersonatedSigner } from '@test/helpers';
10+
import { getImpersonatedSigner, validateArraysEqual } from '@test/helpers';
1111
import { tribeCouncilPodConfig, PodCreationConfig } from '@protocol/optimisticGovernance';
1212
import { abi as inviteTokenABI } from '../../artifacts/@orcaprotocol/contracts/contracts/InviteToken.sol/InviteToken.json';
1313
import { abi as timelockABI } from '../../artifacts/@openzeppelin/contracts/governance/TimelockController.sol/TimelockController.json';
1414
import { abi as gnosisSafeABI } from '../../artifacts/contracts/pods/interfaces/IGnosisSafe.sol/IGnosisSafe.json';
1515
import { Contract } from 'ethers';
1616
import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers';
1717

18-
const validateArraysEqual = (arrayA: string[], arrayB: string[]) => {
19-
arrayA.every((a) => expect(arrayB.map((b) => b.toLowerCase()).includes(a.toLowerCase())));
20-
};
21-
2218
// Transfers Orca tokens from deployer address to the factory, so that it can deploy pods
2319
// Requirement of holding Orca tokens to deploy is a slow rollout mechanism used by Orca
2420
const transferOrcaTokens = async (

‎proposals/dao/fip_82b.ts‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
ValidateUpgradeFunc
1010
} from '@custom-types/types';
1111
import { Contract } from 'ethers';
12+
import { validateArraysEqual } from '@test/helpers';
1213

1314
/*
1415
@@ -26,9 +27,17 @@ const fipNumber = 'fip_82b';
2627
// Do any deployments
2728
// This should exclusively include new contract deployments
2829
const deploy: DeployUpgradeFunc = async (deployAddress: string, addresses: NamedAddresses, logging: boolean) => {
29-
console.log(`No deploy actions for fip${fipNumber}`);
30+
const deploySigner = (await ethers.getSigners())[0];
31+
const previousPCVGuardian = await ethers.getContractAt('PCVGuardian', addresses.pcvGuardian, deploySigner);
32+
const safeAddresses = await previousPCVGuardian.getSafeAddresses();
33+
34+
const pcvGuardianFactory = await ethers.getContractFactory('PCVGuardian');
35+
const pcvGuardianNew = await pcvGuardianFactory.deploy(addresses.core, safeAddresses);
36+
logging && console.log('PCVGuardian deployed to: ', pcvGuardianNew.address);
37+
logging && console.log('PCVGuardian safeAddresses: ', safeAddresses);
38+
3039
return {
31-
// put returned contract objects here
40+
pcvGuardianNew
3241
};
3342
};
3443

@@ -48,7 +57,40 @@ const teardown: TeardownUpgradeFunc = async (addresses, oldContracts, contracts,
4857
// Run any validations required on the fip using mocha or console logging
4958
// IE check balances, check state of contracts, etc.
5059
const validate: ValidateUpgradeFunc = async (addresses, oldContracts, contracts, logging) => {
51-
console.log(`No actions to complete in validate for fip${fipNumber}`);
60+
// 1. Validate new PCVGuardian deployment and roles granted
61+
const core = contracts.core;
62+
const newPCVGuardian = contracts.pcvGuardianNew;
63+
64+
const safeAddresses = await newPCVGuardian.getSafeAddresses();
65+
const expectedSafeAddresses = [
66+
'0xd51dba7a94e1adea403553a8235c302cebf41a3c',
67+
'0x4fcb1435fd42ce7ce7af3cb2e98289f79d2962b3',
68+
'0x98e5f5706897074a4664dd3a32eb80242d6e694b',
69+
'0x5b86887e171bae0c2c826e87e34df8d558c079b9',
70+
'0x2a188f9eb761f70ecea083ba6c2a40145078dfc2',
71+
'0xb0e731f036adfdec12da77c15aab0f90e8e45a0e',
72+
'0x24f663c69cd4b263cf5685a49013ff5f1c898d24',
73+
'0x5ae217de26f6ff5f481c6e10ec48b2cf2fc857c8',
74+
'0xe8633c49ace655eb4a8b720e6b12f09bd3a97812',
75+
'0xcd1ac0014e2ebd972f40f24df1694e6f528b2fd4',
76+
'0xc5bb8f0253776bec6ff450c2b40f092f7e7f5b57',
77+
'0xc4eac760c2c631ee0b064e39888b89158ff808b2',
78+
'0x2c47fef515d2c70f2427706999e158533f7cf090',
79+
'0x9aadffe00eae6d8e59bb4f7787c6b99388a6960d',
80+
'0xd2174d78637a40448112aa6b30f9b19e6cf9d1f9',
81+
'0x5dde9b4b14edf59cb23c1d4579b279846998205e'
82+
];
83+
validateArraysEqual(safeAddresses, expectedSafeAddresses);
84+
85+
// New PCV Guardian roles - validate granted
86+
expect(await core.hasRole(ethers.utils.id('GUARDIAN_ROLE'), newPCVGuardian.address)).to.be.true;
87+
expect(await core.hasRole(ethers.utils.id('PCV_CONTROLLER_ROLE'), newPCVGuardian.address)).to.be.true;
88+
89+
// Old PCV Guardian roles - validate revoked
90+
expect(await core.hasRole(ethers.utils.id('GUARDIAN_ROLE'), addresses.pcvGuardian)).to.be.false;
91+
expect(await core.hasRole(ethers.utils.id('PCV_CONTROLLER_ROLE'), addresses.pcvGuardian)).to.be.false;
92+
93+
// 2. Validate TribalCouncil role transfers
5294
await validateTransferredRoleAdmins(contracts.core);
5395
await validateNewCouncilRoles(contracts.core);
5496
await validateContractAdmins(contracts);
@@ -76,6 +118,7 @@ const validateTransferredRoleAdmins = async (core: Contract) => {
76118
expect(await core.getRoleAdmin(ethers.utils.id('METAGOVERNANCE_GAUGE_ADMIN'))).to.be.equal(ROLE_ADMIN);
77119
expect(await core.getRoleAdmin(ethers.utils.id('SWAP_ADMIN_ROLE'))).to.be.equal(ROLE_ADMIN);
78120
expect(await core.getRoleAdmin(ethers.utils.id('VOTIUM_ADMIN_ROLE'))).to.be.equal(ROLE_ADMIN);
121+
expect(await core.getRoleAdmin(ethers.utils.id('RATE_LIMITED_MINTER_ADMIN'))).to.be.equal(ROLE_ADMIN);
79122
};
80123

81124
/// Validate that the expected new TribeRoles have been created
@@ -86,6 +129,8 @@ const validateNewCouncilRoles = async (core: Contract) => {
86129
expect(await core.getRoleAdmin(ethers.utils.id('FEI_MINT_ADMIN'))).to.be.equal(ROLE_ADMIN);
87130
expect(await core.getRoleAdmin(ethers.utils.id('PCV_MINOR_PARAM_ROLE'))).to.be.equal(ROLE_ADMIN);
88131
expect(await core.getRoleAdmin(ethers.utils.id('PSM_ADMIN_ROLE'))).to.be.equal(ROLE_ADMIN);
132+
expect(await core.getRoleAdmin(ethers.utils.id('PCV_SAFE_MOVER_ROLE'))).to.be.equal(ROLE_ADMIN);
133+
expect(await core.getRoleAdmin(ethers.utils.id('PCV_GUARDIAN_ADMIN_ROLE'))).to.be.equal(ROLE_ADMIN);
89134
};
90135

91136
/// Validate that the relevant contract admins have been set to their expected values
@@ -134,6 +179,7 @@ const validateTribalCouncilRoles = async (core: Contract, tribalCouncilTimelockA
134179
expect(await core.hasRole(ethers.utils.id('PCV_GUARDIAN_ADMIN_ROLE'), tribalCouncilTimelockAddress)).to.be.true;
135180
expect(await core.hasRole(ethers.utils.id('ORACLE_ADMIN_ROLE'), tribalCouncilTimelockAddress)).to.be.true;
136181
expect(await core.hasRole(ethers.utils.id('PSM_ADMIN_ROLE'), tribalCouncilTimelockAddress)).to.be.true;
182+
expect(await core.hasRole(ethers.utils.id('PCV_SAFE_MOVER_ROLE'), tribalCouncilTimelockAddress)).to.be.true;
137183
};
138184

139185
export { deploy, setup, teardown, validate };

‎proposals/description/fip_82b.ts‎

Lines changed: 71 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,16 @@ const fip_82b: ProposalDescription = {
165165
],
166166
description: 'Create MINOR_PARAM_ROLE role, assigning ROLE_ADMIN as the role admin'
167167
},
168+
{
169+
target: 'core',
170+
values: '0',
171+
method: 'createRole(bytes32,bytes32)',
172+
arguments: [
173+
'0xd387c7eec7161b3637e694ef5cf8f1a9e29bfd35135c86c9b540bebec4ef221a', // PCV_SAFE_MOVER_ROLE
174+
'0x2172861495e7b85edac73e3cd5fbb42dd675baadf627720e687bcfdaca025096' // ROLE_ADMIN
175+
],
176+
description: 'Create PCV_SAFE_MOVER_ROLE role, assigning ROLE_ADMIN as the role admin'
177+
},
168178
/////// Set the relevant contract admins to the newly created roles //////////
169179
// FUSE_ADMIN
170180
{
@@ -298,16 +308,6 @@ const fip_82b: ProposalDescription = {
298308
],
299309
description: 'Grant TribalCouncil timelock the PCV_MINOR_PARAM_ROLE role'
300310
},
301-
{
302-
target: 'core',
303-
values: '0',
304-
method: 'grantRole(bytes32,address)',
305-
arguments: [
306-
'0xdf6ce05acd28d71e96472375ef55c4a692f167af3ccda9500570f7373c1ba22c', // PCV_GUARDIAN_ADMIN_ROLE
307-
'{tribalCouncilTimelock}'
308-
],
309-
description: 'Grant TribalCouncil timelock the PCV_GUARDIAN_ADMIN_ROLE role'
310-
},
311311
{
312312
target: 'core',
313313
values: '0',
@@ -388,6 +388,67 @@ const fip_82b: ProposalDescription = {
388388
'{optimisticTimelock}'
389389
],
390390
description: 'Grant optimisticTimelock the TOKEMAK_DEPOSIT_ADMIN_ROLE role'
391+
},
392+
{
393+
target: 'core',
394+
values: '0',
395+
method: 'grantRole(bytes32,address)',
396+
arguments: [
397+
'0xd387c7eec7161b3637e694ef5cf8f1a9e29bfd35135c86c9b540bebec4ef221a', // PCV_SAFE_MOVER_ROLE
398+
'{tribalCouncilTimelock}'
399+
],
400+
description: 'Grant tribalCouncilTimelock the PCV_SAFE_MOVER_ROLE role'
401+
},
402+
{
403+
target: 'core',
404+
values: '0',
405+
method: 'grantRole(bytes32,address)',
406+
arguments: [
407+
'0xdf6ce05acd28d71e96472375ef55c4a692f167af3ccda9500570f7373c1ba22c', // PCV_GUARDIAN_ADMIN_ROLE
408+
'{tribalCouncilTimelock}'
409+
],
410+
description: 'Grant tribalCouncilTimelock the PCV_GUARDIAN_ADMIN_ROLE role'
411+
},
412+
////// Authorise new PCV Guardian and revoke old PCV Guardian ////////
413+
{
414+
target: 'core',
415+
values: '0',
416+
method: 'grantRole(bytes32,address)',
417+
arguments: [
418+
'0x0866eae1216ed05a11636a648003f3f62921eb97ccb05acc30636f62958a8bd6', // PCV_CONTROLLER
419+
'{pcvGuardianNew}'
420+
],
421+
description: 'Grant newPCVGuardian the PCV_CONTROLLER_ROLE'
422+
},
423+
{
424+
target: 'core',
425+
values: '0',
426+
method: 'grantRole(bytes32,address)',
427+
arguments: [
428+
'0x55435dd261a4b9b3364963f7738a7a662ad9c84396d64be3365284bb7f0a5041', // GUARDIAN
429+
'{pcvGuardianNew}'
430+
],
431+
description: 'Grant newPCVGuardian the GUARDIAN role'
432+
},
433+
{
434+
target: 'core',
435+
values: '0',
436+
method: 'revokeRole(bytes32,address)',
437+
arguments: [
438+
'0x0866eae1216ed05a11636a648003f3f62921eb97ccb05acc30636f62958a8bd6', // PCV_CONTROLLER
439+
'{pcvGuardian}'
440+
],
441+
description: 'Revoke PCV_CONTROLLER from old pcvGuardian'
442+
},
443+
{
444+
target: 'core',
445+
values: '0',
446+
method: 'revokeRole(bytes32,address)',
447+
arguments: [
448+
'0x55435dd261a4b9b3364963f7738a7a662ad9c84396d64be3365284bb7f0a5041', // GUARDIAN
449+
'{pcvGuardian}'
450+
],
451+
description: 'Revoke GUARDIAN from old pcvGuardian'
391452
}
392453
],
393454
description: `

‎protocol-configuration/permissions.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,14 @@ export const permissions = {
1717
'feiDAOTimelock',
1818
'ratioPCVControllerV2',
1919
'aaveEthPCVDripController',
20-
'pcvGuardian',
20+
'pcvGuardianNew',
2121
'daiPCVDripController',
2222
'lusdPCVDripController',
2323
'ethPSMFeiSkimmer',
2424
'lusdPSMFeiSkimmer',
2525
'raiPCVDripController'
2626
],
27-
GUARDIAN_ROLE: ['multisig', 'pcvGuardian', 'pcvSentinel'],
27+
GUARDIAN_ROLE: ['multisig', 'pcvGuardianNew', 'pcvSentinel'],
2828
ORACLE_ADMIN_ROLE: [
2929
'collateralizationOracleGuardian',
3030
'optimisticTimelock',
@@ -38,6 +38,7 @@ export const permissions = {
3838
FUSE_ADMIN: ['optimisticTimelock', 'tribalChiefSyncV2', 'tribalCouncilTimelock'],
3939
VOTIUM_ADMIN_ROLE: ['opsOptimisticTimelock'],
4040
PCV_GUARDIAN_ADMIN_ROLE: ['optimisticTimelock', 'tribalCouncilTimelock'],
41+
PCV_SAFE_MOVER_ROLE: ['tribalCouncilTimelock'],
4142
METAGOVERNANCE_VOTE_ADMIN: ['feiDAOTimelock', 'opsOptimisticTimelock'],
4243
METAGOVERNANCE_TOKEN_STAKING: ['feiDAOTimelock', 'opsOptimisticTimelock'],
4344
METAGOVERNANCE_GAUGE_ADMIN: ['feiDAOTimelock', 'optimisticTimelock'],

‎test/helpers.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ async function getCore(): Promise<Core> {
134134
return core;
135135
}
136136

137+
const validateArraysEqual = (arrayA: string[], arrayB: string[]) => {
138+
expect(arrayA.length).to.equal(arrayB.length);
139+
arrayA.every((a) => expect(arrayB.map((b) => b.toLowerCase()).includes(a.toLowerCase())));
140+
};
141+
137142
async function expectApprox(
138143
actual: string | number | BigNumberish,
139144
expected: string | number | BigNumberish,
@@ -343,5 +348,6 @@ export {
343348
resetFork,
344349
overwriteChainlinkAggregator,
345350
performDAOAction,
346-
initialiseGnosisSDK
351+
initialiseGnosisSDK,
352+
validateArraysEqual
347353
};

0 commit comments

Comments
 (0)