Skip to content

Commit da3c1bb

Browse files
committed
feat: use more granular timelock roles
1 parent 49159ca commit da3c1bb

File tree

4 files changed

+42
-27
lines changed

4 files changed

+42
-27
lines changed

contracts/dao/governor/FeiDAO.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ contract FeiDAO is
139139
function getVotes(address account, uint256 blockNumber)
140140
public
141141
view
142-
override(IGovernor, GovernorVotesComp)
142+
override(Governor, IGovernor)
143143
returns (uint256)
144144
{
145145
return super.getVotes(account, blockNumber);

contracts/pods/PodFactory.sol

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,10 @@ contract PodFactory is CoreRef, IPodFactory {
268268
/// @param publicExecutor Non-permissioned smart contract that
269269
/// allows any address to execute a ready transaction
270270
/// @param podAdmin Address which is the admin of the Orca pods
271-
/// Will also be able to propose
271+
/// @dev Roles that individual addresses are granted on the relevant timelock:
272+
// safeAddress - PROPOSER_ROLE, CANCELLER_ROLE, EXECUTOR_ROLE
273+
// podAdmin - CANCELLER_ROLE
274+
// publicExecutor - EXECUTOR_ROLE
272275
function _createTimelock(
273276
address safeAddress,
274277
uint256 minDelay,
@@ -288,6 +291,10 @@ contract PodFactory is CoreRef, IPodFactory {
288291
proposers,
289292
executors
290293
);
294+
295+
// Revoke PROPOSER_ROLE priviledges from podAdmin. Only pod Safe can propose
296+
timelock.revokeRole(timelock.PROPOSER_ROLE(), podAdmin);
297+
291298
// Revoke TIMELOCK_ADMIN_ROLE priviledges from deployer factory
292299
timelock.revokeRole(timelock.TIMELOCK_ADMIN_ROLE(), address(this));
293300

contracts/test/integration/governance/PodAdminGateway.t.sol

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: GPL-3.0-or-later
22
pragma solidity ^0.8.0;
33

4+
import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol";
45
import {ControllerV1} from "@orcaprotocol/contracts/contracts/ControllerV1.sol";
56
import {Vm} from "../../utils/Vm.sol";
67
import {DSTest} from "../../utils/DSTest.sol";
@@ -9,7 +10,6 @@ import {PodAdminGateway} from "../../../pods/PodAdminGateway.sol";
910
import {IPodAdminGateway} from "../../../pods/interfaces/IPodAdminGateway.sol";
1011
import {mintOrcaTokens, getPodParamsWithTimelock} from "../fixtures/Orca.sol";
1112
import {IPodFactory} from "../../../pods/interfaces/IPodFactory.sol";
12-
import {ITimelock} from "../../../dao/timelock/ITimelock.sol";
1313
import {TribeRoles} from "../../../core/TribeRoles.sol";
1414
import {ICore} from "../../../core/ICore.sol";
1515
import {MainnetAddresses} from "../fixtures/MainnetAddresses.sol";
@@ -67,18 +67,18 @@ contract PodAdminGatewayIntegrationTest is DSTest {
6767
address podAdmin = factory.getPodAdmin(podId);
6868
assertEq(podAdmin, address(podAdminGateway));
6969

70-
// Validate VetoController has proposer role, this will allow it to veto
71-
ITimelock timelockContract = ITimelock(timelock);
72-
bool hasProposerRole = timelockContract.hasRole(
73-
keccak256("PROPOSER_ROLE"),
74-
address(address(podAdminGateway))
70+
// Validate PodAdminGateway has CANCELLER role, this will allow it to veto
71+
TimelockController timelockContract = TimelockController(
72+
payable(timelock)
7573
);
76-
assertTrue(hasProposerRole);
77-
78-
bool memberTransfersLocked = factory.getIsMembershipTransferLocked(
79-
podId
74+
assertTrue(
75+
timelockContract.hasRole(
76+
timelockContract.CANCELLER_ROLE(),
77+
address(podAdminGateway)
78+
)
8079
);
81-
assertTrue(memberTransfersLocked);
80+
81+
assertTrue(factory.getIsMembershipTransferLocked(podId));
8282
}
8383

8484
/// @notice Validate that a podAdmin can be added for a particular pod by the GOVERNOR

contracts/test/integration/governance/PodFactory.t.sol

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,6 @@ contract PodFactoryIntegrationTest is DSTest {
2626
PodExecutor podExecutor;
2727
address private podAdmin;
2828

29-
bytes32 public constant PROPOSER_ROLE = keccak256("PROPOSER_ROLE");
30-
bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE");
31-
3229
address core = MainnetAddresses.CORE;
3330
address memberToken = MainnetAddresses.MEMBER_TOKEN;
3431
address podController = MainnetAddresses.POD_CONTROLLER;
@@ -225,21 +222,32 @@ contract PodFactoryIntegrationTest is DSTest {
225222
payable(timelock)
226223
);
227224

228-
// Gnosis safe should be the proposer
229-
bool hasProposerRole = timelockContract.hasRole(PROPOSER_ROLE, safe);
230-
assertTrue(hasProposerRole);
225+
// Gnosis safe should be: PROPOSER, EXECUTOR, CANCELLOR
226+
assertTrue(
227+
timelockContract.hasRole(timelockContract.PROPOSER_ROLE(), safe)
228+
);
229+
assertTrue(
230+
timelockContract.hasRole(timelockContract.EXECUTOR_ROLE(), safe)
231+
);
232+
assertTrue(
233+
timelockContract.hasRole(timelockContract.CANCELLER_ROLE(), safe)
234+
);
231235

232-
bool safeAddressIsExecutor = timelockContract.hasRole(
233-
EXECUTOR_ROLE,
234-
safe
236+
// PodExecutor should be: EXECUTOR
237+
assertTrue(
238+
timelockContract.hasRole(
239+
timelockContract.EXECUTOR_ROLE(),
240+
address(podExecutor)
241+
)
235242
);
236-
assertTrue(safeAddressIsExecutor);
237243

238-
bool publicPodExecutorIsExecutor = timelockContract.hasRole(
239-
EXECUTOR_ROLE,
240-
address(podExecutor)
244+
// PodAdmin should be: CANCELLOR
245+
assertTrue(
246+
timelockContract.hasRole(
247+
timelockContract.CANCELLER_ROLE(),
248+
podAdmin
249+
)
241250
);
242-
assertTrue(publicPodExecutorIsExecutor);
243251

244252
// Min delay of timelock
245253
assertEq(timelockContract.getMinDelay(), podConfig.minDelay);

0 commit comments

Comments
 (0)