Skip to content
This repository has been archived by the owner on Nov 27, 2024. It is now read-only.

Commit

Permalink
made changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@dd0sxx
dd0sxx committed Jan 9, 2024
1 parent c0a7d0f commit 14a2ab7
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 51 deletions.
40 changes: 22 additions & 18 deletions src/guards/set-role-holders/SetRoleHoldersGuard.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity 0.8.19;
import {ILlamaActionGuard} from "src/interfaces/ILlamaActionGuard.sol";
import {LlamaUtils} from "src/lib/LlamaUtils.sol";
import {ActionInfo, RoleHolderData} from "src/lib/Structs.sol";
import {AuthorizeSetRoleHolderData} from "src/guards/set-role-holders/SetRoleHoldersGuardFactory.sol";

/// @title Set Role Holders Guard
/// @author Llama (devsdosomething@llama.xyz)
Expand Down Expand Up @@ -31,9 +32,6 @@ contract SetRoleHoldersGuard is ILlamaActionGuard {
// ======== Storage Variables ========
// ===================================

/// @notice BYPASS_PROTECTION_ROLE can be set to 0 to disable this feature.
/// This also means the all holders role cannot be set as the BYPASS_PROTECTION_ROLE.
uint8 public immutable BYPASS_PROTECTION_ROLE;
/// @notice The `LlamaExecutor` contract address that controls this guard contract.
address public immutable EXECUTOR;

Expand All @@ -44,9 +42,9 @@ contract SetRoleHoldersGuard is ILlamaActionGuard {
// ======== Contract Creation ========
// ===================================

constructor(uint8 _BYPASS_PROTECTION_ROLE, address _executor) {
BYPASS_PROTECTION_ROLE = _BYPASS_PROTECTION_ROLE;
constructor(address _executor, AuthorizeSetRoleHolderData[] memory authorizeSetRoleHolderData) {
EXECUTOR = _executor;
_setAuthorizedSetRoleHolder(authorizeSetRoleHolderData);
}

// ================================
Expand All @@ -56,30 +54,36 @@ contract SetRoleHoldersGuard is ILlamaActionGuard {
/// @inheritdoc ILlamaActionGuard
/// @notice Performs a validation check at action creation time that the action creator is authorized to set the role.
function validateActionCreation(ActionInfo calldata actionInfo) external view {
if (BYPASS_PROTECTION_ROLE == 0 || actionInfo.creatorRole != BYPASS_PROTECTION_ROLE) {
RoleHolderData[] memory roleHolderData = abi.decode(actionInfo.data[4:], (RoleHolderData[])); // skip selector
uint256 length = roleHolderData.length;
for (uint256 i = 0; i < length; LlamaUtils.uncheckedIncrement(i)) {
if (!authorizedSetRoleHolder[actionInfo.creatorRole][roleHolderData[i].role]) {
revert UnauthorizedSetRoleHolder(actionInfo.creatorRole, roleHolderData[i].role);
}
RoleHolderData[] memory roleHolderData = abi.decode(actionInfo.data[4:], (RoleHolderData[])); // skip selector
uint256 length = roleHolderData.length;
for (uint256 i = 0; i < length; LlamaUtils.uncheckedIncrement(i)) {
if (!authorizedSetRoleHolder[actionInfo.creatorRole][roleHolderData[i].role]) {
revert UnauthorizedSetRoleHolder(actionInfo.creatorRole, roleHolderData[i].role);
}
}
}

/// @notice Allows the EXECUTOR to set the authorizedSetRoleHolder mapping.
/// @param actionCreatorRole The role that is is being authorized or unauthorized to set the targetRole.
/// @param targetRole The role that the actionCreatorRole is being authorized or unauthorized to set.
/// @param isAuthorized Whether the actionCreatorRole is authorized to set the targetRole.
function setAuthorizedSetRoleHolder(uint8 actionCreatorRole, uint8 targetRole, bool isAuthorized) external {
/// @param authorizeSetRoleHolderData The data to set the authorizedSetRoleHolder mapping.
function setAuthorizedSetRoleHolder(AuthorizeSetRoleHolderData[] memory authorizeSetRoleHolderData) external {
if (msg.sender != EXECUTOR) revert OnlyLlamaExecutor();
authorizedSetRoleHolder[actionCreatorRole][targetRole] = isAuthorized;
emit AuthorizedSetRoleHolder(actionCreatorRole, targetRole, isAuthorized);
_setAuthorizedSetRoleHolder(authorizeSetRoleHolderData);
}

/// @inheritdoc ILlamaActionGuard
function validatePreActionExecution(ActionInfo calldata actionInfo) external pure {}

/// @inheritdoc ILlamaActionGuard
function validatePostActionExecution(ActionInfo calldata actionInfo) external pure {}

// ================================
// ======== Internal Logic ========
// ================================
function _setAuthorizedSetRoleHolder(AuthorizeSetRoleHolderData[] memory data) internal {
uint256 length = data.length;
for (uint256 i = 0; i < length; LlamaUtils.uncheckedIncrement(i)) {
authorizedSetRoleHolder[data[i].actionCreatorRole][data[i].targetRole] = data[i].isAuthorized;
emit AuthorizedSetRoleHolder(data[i].actionCreatorRole, data[i].targetRole, data[i].isAuthorized);
}
}
}
18 changes: 11 additions & 7 deletions src/guards/set-role-holders/SetRoleHoldersGuardFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,29 @@ pragma solidity 0.8.19;

import {SetRoleHoldersGuard} from "src/guards/set-role-holders/SetRoleHoldersGuard.sol";

struct AuthorizeSetRoleHolderData {
uint8 actionCreatorRole;
uint8 targetRole;
bool isAuthorized;
}

/// @title Set Role Holders Guard Factory
/// @author Llama (devsdosomething@llama.xyz)
/// @notice A factory contract that deploys `SetRoleHoldersGuard` contracts.
/// The `SetRoleHoldersGuard` contract is used to specify which roles are allowed to set other roles, by setting a guard
/// on the `setRoleHolders` function in the `LlamaGovernanceScript` contract.
contract SetRoleHoldersGuardFactory {
/// @notice Emitted when a new `SetRoleHoldersGuard` contract is deployed.
event SetRoleHoldersGuardCreated(
address indexed deployer, address indexed executor, address guard, uint8 bypassProtectionRole
);
event SetRoleHoldersGuardCreated(address indexed deployer, address indexed executor, address guard);

/// @notice Deploys a new `SetRoleHoldersGuard` contract.
/// @param bypassProtectionRole The role that can bypass the protection.
/// @param executor The address of the executor contract.
function deploySetRoleHoldersGuard(uint8 bypassProtectionRole, address executor)
/// @param authorizeSetRoleHolderData The initial role authorizations.
function deploySetRoleHoldersGuard(address executor, AuthorizeSetRoleHolderData[] memory authorizeSetRoleHolderData)
external
returns (SetRoleHoldersGuard guard)
{
guard = new SetRoleHoldersGuard(bypassProtectionRole, executor);
emit SetRoleHoldersGuardCreated(msg.sender, executor, address(guard), bypassProtectionRole);
guard = new SetRoleHoldersGuard(executor, authorizeSetRoleHolderData);
emit SetRoleHoldersGuardCreated(msg.sender, executor, address(guard));
}
}
49 changes: 23 additions & 26 deletions test/guards/SetRoleHoldersGuard.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ pragma solidity ^0.8.19;
import {Test, console2} from "forge-std/Test.sol";

import {SetRoleHoldersGuard} from "src/guards/set-role-holders/SetRoleHoldersGuard.sol";
import {SetRoleHoldersGuardFactory} from "src/guards/set-role-holders/SetRoleHoldersGuardFactory.sol";
import {
SetRoleHoldersGuardFactory,
AuthorizeSetRoleHolderData
} from "src/guards/set-role-holders/SetRoleHoldersGuardFactory.sol";
import {ActionInfo, PermissionData, RoleHolderData} from "src/lib/Structs.sol";
import {LlamaGovernanceScript} from "src/llama-scripts/LlamaGovernanceScript.sol";
import {Roles, LlamaTestSetup} from "test/utils/LlamaTestSetup.sol";
Expand All @@ -19,7 +22,8 @@ contract SetRoleHolderTest is LlamaGovernanceScriptTest {
function setUp() public override {
LlamaGovernanceScriptTest.setUp();
setRoleHoldersGuardFactory = new SetRoleHoldersGuardFactory();
guard = setRoleHoldersGuardFactory.deploySetRoleHoldersGuard(uint8(0), address(mpExecutor));
guard =
setRoleHoldersGuardFactory.deploySetRoleHoldersGuard(address(mpExecutor), new AuthorizeSetRoleHolderData[](0));
vm.prank(address(mpExecutor));
mpCore.setGuard(address(govScript), SET_ROLE_HOLDERS_SELECTOR, guard);
}
Expand All @@ -34,7 +38,7 @@ contract ValidateActionCreation is SetRoleHolderTest {

bytes memory data = abi.encodeWithSelector(SET_ROLE_HOLDERS_SELECTOR, roleHolderData);

// There is no bypass role, and we have not set any authorizations, so this should always revert.
// We have not set any authorizations, so this should always revert.
vm.expectRevert(
abi.encodeWithSelector(
SetRoleHoldersGuard.UnauthorizedSetRoleHolder.selector, uint8(Roles.ActionCreator), targetRole
Expand All @@ -44,34 +48,20 @@ contract ValidateActionCreation is SetRoleHolderTest {
mpCore.createAction(uint8(Roles.ActionCreator), mpStrategy2, address(govScript), 0, data, "");
}

function test_BypassProtectionRoleWorksWithAllExisingRoles(uint8 targetRole) public {
targetRole = uint8(bound(targetRole, 1, 8)); // number of existing roles excluding all holders role

// create a new guard with a bypass role
guard = setRoleHoldersGuardFactory.deploySetRoleHoldersGuard(uint8(Roles.ActionCreator), address(mpExecutor));
vm.prank(address(mpExecutor));
mpCore.setGuard(address(govScript), SET_ROLE_HOLDERS_SELECTOR, guard);

RoleHolderData[] memory roleHolderData = new RoleHolderData[](1);
roleHolderData[0] =
RoleHolderData({role: targetRole, policyholder: approverAdam, quantity: 1, expiration: type(uint64).max});

bytes memory data = abi.encodeWithSelector(SET_ROLE_HOLDERS_SELECTOR, roleHolderData);

ActionInfo memory actionInfo = _createAndApproveAndQueueAction(data);
mpCore.executeAction(actionInfo);

assertEq(mpPolicy.hasRole(approverAdam, targetRole), true);
}

function test_AuthorizedSetRoleHolder(uint8 targetRole) public {
targetRole = uint8(bound(targetRole, 1, 8)); // number of existing roles excluding all holders role

// set role authorization
vm.prank(address(mpExecutor));
vm.expectEmit();
AuthorizeSetRoleHolderData[] memory authorizeSetRoleHolderData = new AuthorizeSetRoleHolderData[](1);
authorizeSetRoleHolderData[0] = AuthorizeSetRoleHolderData({
actionCreatorRole: uint8(Roles.ActionCreator),
targetRole: targetRole,
isAuthorized: true
});
emit AuthorizedSetRoleHolder(uint8(Roles.ActionCreator), targetRole, true);
guard.setAuthorizedSetRoleHolder(uint8(Roles.ActionCreator), targetRole, true);
guard.setAuthorizedSetRoleHolder(authorizeSetRoleHolderData);

RoleHolderData[] memory roleHolderData = new RoleHolderData[](1);
roleHolderData[0] =
Expand All @@ -92,8 +82,14 @@ contract ValidateActionCreation is SetRoleHolderTest {
// set role authorization
vm.prank(address(mpExecutor));
vm.expectEmit();
AuthorizeSetRoleHolderData[] memory authorizeSetRoleHolderData = new AuthorizeSetRoleHolderData[](1);
authorizeSetRoleHolderData[0] = AuthorizeSetRoleHolderData({
actionCreatorRole: uint8(Roles.ActionCreator),
targetRole: targetRole,
isAuthorized: true
});
emit AuthorizedSetRoleHolder(uint8(Roles.ActionCreator), targetRole, true);
guard.setAuthorizedSetRoleHolder(uint8(Roles.ActionCreator), targetRole, true);
guard.setAuthorizedSetRoleHolder(authorizeSetRoleHolderData);

RoleHolderData[] memory roleHolderData = new RoleHolderData[](1);
roleHolderData[0] =
Expand All @@ -106,8 +102,9 @@ contract ValidateActionCreation is SetRoleHolderTest {
// setting role authorization to false mid action
vm.prank(address(mpExecutor));
vm.expectEmit();
authorizeSetRoleHolderData[0].isAuthorized = false;
emit AuthorizedSetRoleHolder(uint8(Roles.ActionCreator), targetRole, false);
guard.setAuthorizedSetRoleHolder(uint8(Roles.ActionCreator), targetRole, false);
guard.setAuthorizedSetRoleHolder(authorizeSetRoleHolderData);

mpCore.executeAction(actionInfo);

Expand Down

0 comments on commit 14a2ab7

Please sign in to comment.