Skip to content

Commit

Permalink
formatting: First round of cleanup (#21)
Browse files Browse the repository at this point in the history 
* refactor to simplify into 1 contract and interface

* refactor guardian to be part of access control, also queue becomes a separate role

* formatting

* feat: update with new cleanup

* feat: add section labels, reorder

* chore: reduce diff

---------

Co-authored-by: Sam MacPherson <[email protected]>
  • Loading branch information
@lucas-manuel @hexonaut
lucas-manuel and hexonaut authored Jun 21, 2024
1 parent 4336079 commit 9374feb
Show file tree
Hide file tree
Showing 4 changed files with 251 additions and 224 deletions.
205 changes: 97 additions & 108 deletions src/Executor.sol
View file
Original file line number Diff line number Diff line change
@@ -1,54 +1,53 @@
// SPDX-License-Identifier: AGPL-3.0
pragma solidity ^0.8.10;
pragma solidity ^0.8.22;

import { AccessControl } from "openzeppelin-contracts/contracts/access/AccessControl.sol";
import { Address } from "openzeppelin-contracts/contracts/utils/Address.sol";

import { IExecutor } from './interfaces/IExecutor.sol';

/**
* @title Executor
* @title Executor
* @author Aave
* @notice Executor which queues up message calls and executes them after an optional delay
*/
contract Executor is IExecutor, AccessControl {

using Address for address;

/******************************************************************************************************************/
/*** State variables and constructor ***/
/******************************************************************************************************************/

bytes32 public constant SUBMISSION_ROLE = keccak256('SUBMISSION_ROLE');
bytes32 public constant GUARDIAN_ROLE = keccak256('GUARDIAN_ROLE');

// Minimum allowed grace period, which reduces the risk of having an actions set expire due to network congestion
uint256 constant MINIMUM_GRACE_PERIOD = 10 minutes;

// Time between queuing and execution
uint256 private _delay;
// Time after the execution time during which the actions set can be executed
uint256 private _gracePeriod;
uint256 public constant MINIMUM_GRACE_PERIOD = 10 minutes;

// Number of actions sets
uint256 private _actionsSetCounter;
// Map of registered actions sets (id => ActionsSet)
mapping(uint256 => ActionsSet) private _actionsSets;
// Map of queued actions (actionHash => isQueued)
mapping(bytes32 => bool) private _queuedActions;

uint256 public override actionsSetCount; // Number of actions sets
uint256 public override delay; // Time between queuing and execution
uint256 public override gracePeriod; // Time after delay during which an actions set can be executed

mapping(bytes32 => bool) public override isActionQueued;

/**
* @dev Constructor
*
* @param delay The delay before which an actions set can be executed
* @param gracePeriod The time period after a delay during which an actions set can be executed
* @dev Constructor
* @param delay_ The delay before which an actions set can be executed.
* @param gracePeriod_ The time period after a delay during which an actions set can be executed.
*/
constructor(
uint256 delay,
uint256 gracePeriod
uint256 delay_,
uint256 gracePeriod_
) {
if (
gracePeriod < MINIMUM_GRACE_PERIOD
gracePeriod_ < MINIMUM_GRACE_PERIOD
) revert InvalidInitParams();

_updateDelay(delay);
_updateGracePeriod(gracePeriod);
_updateDelay(delay_);
_updateGracePeriod(gracePeriod_);

_setRoleAdmin(SUBMISSION_ROLE, DEFAULT_ADMIN_ROLE);
_setRoleAdmin(GUARDIAN_ROLE, DEFAULT_ADMIN_ROLE);
Expand All @@ -57,6 +56,10 @@ contract Executor is IExecutor, AccessControl {
_grantRole(DEFAULT_ADMIN_ROLE, address(this)); // Necessary for self-referential calls to change configuration
}

/******************************************************************************************************************/
/*** ActionSet functions ***/
/******************************************************************************************************************/

/// @inheritdoc IExecutor
function queue(
address[] memory targets,
Expand All @@ -66,21 +69,21 @@ contract Executor is IExecutor, AccessControl {
bool[] memory withDelegatecalls
) external override onlyRole(SUBMISSION_ROLE) {
if (targets.length == 0) revert EmptyTargets();

uint256 targetsLength = targets.length;
if (
targetsLength != values.length ||
targetsLength != values.length ||
targetsLength != signatures.length ||
targetsLength != calldatas.length ||
targetsLength != calldatas.length ||
targetsLength != withDelegatecalls.length
) revert InconsistentParamsLength();

uint256 actionsSetId = _actionsSetCounter;
uint256 executionTime = block.timestamp + _delay;
unchecked {
++_actionsSetCounter;
}
uint256 actionsSetId = actionsSetCount;
uint256 executionTime = block.timestamp + delay;

unchecked { ++actionsSetCount; }

for (uint256 i = 0; i < targetsLength; ) {
for (uint256 i = 0; i < targetsLength; ++i) {
bytes32 actionHash = keccak256(
abi.encode(
targets[i],
Expand All @@ -91,20 +94,19 @@ contract Executor is IExecutor, AccessControl {
withDelegatecalls[i]
)
);
if (isActionQueued(actionHash)) revert DuplicateAction();
_queuedActions[actionHash] = true;
unchecked {
++i;
}
if (isActionQueued[actionHash]) revert DuplicateAction();

isActionQueued[actionHash] = true;
}

ActionsSet storage actionsSet = _actionsSets[actionsSetId];
actionsSet.targets = targets;
actionsSet.values = values;
actionsSet.signatures = signatures;
actionsSet.calldatas = calldatas;

actionsSet.targets = targets;
actionsSet.values = values;
actionsSet.signatures = signatures;
actionsSet.calldatas = calldatas;
actionsSet.withDelegatecalls = withDelegatecalls;
actionsSet.executionTime = executionTime;
actionsSet.executionTime = executionTime;

emit ActionsSetQueued(
actionsSetId,
Expand All @@ -122,13 +124,15 @@ contract Executor is IExecutor, AccessControl {
if (getCurrentState(actionsSetId) != ActionsSetState.Queued) revert OnlyQueuedActions();

ActionsSet storage actionsSet = _actionsSets[actionsSetId];

if (block.timestamp < actionsSet.executionTime) revert TimelockNotFinished();

actionsSet.executed = true;

uint256 actionCount = actionsSet.targets.length;

bytes[] memory returnedData = new bytes[](actionCount);
for (uint256 i = 0; i < actionCount; ) {
for (uint256 i = 0; i < actionCount; ++i) {
returnedData[i] = _executeTransaction(
actionsSet.targets[i],
actionsSet.values[i],
Expand All @@ -137,9 +141,6 @@ contract Executor is IExecutor, AccessControl {
actionsSet.executionTime,
actionsSet.withDelegatecalls[i]
);
unchecked {
++i;
}
}

emit ActionsSetExecuted(actionsSetId, msg.sender, returnedData);
Expand All @@ -153,34 +154,41 @@ contract Executor is IExecutor, AccessControl {
actionsSet.canceled = true;

uint256 targetsLength = actionsSet.targets.length;
for (uint256 i = 0; i < targetsLength; ) {
_cancelTransaction(
for (uint256 i = 0; i < targetsLength; ++i) {
_removeActionFromQueue(
actionsSet.targets[i],
actionsSet.values[i],
actionsSet.signatures[i],
actionsSet.calldatas[i],
actionsSet.executionTime,
actionsSet.withDelegatecalls[i]
);
unchecked {
++i;
}
}

emit ActionsSetCanceled(actionsSetId);
}

/******************************************************************************************************************/
/*** Admin functions ***/
/******************************************************************************************************************/

/// @inheritdoc IExecutor
function updateDelay(uint256 delay) external override onlyRole(DEFAULT_ADMIN_ROLE) {
_updateDelay(delay);
function updateDelay(uint256 newDelay) external override onlyRole(DEFAULT_ADMIN_ROLE) {
_updateDelay(newDelay);
}

/// @inheritdoc IExecutor
function updateGracePeriod(uint256 gracePeriod) external override onlyRole(DEFAULT_ADMIN_ROLE) {
if (gracePeriod < MINIMUM_GRACE_PERIOD) revert GracePeriodTooShort();
_updateGracePeriod(gracePeriod);
function updateGracePeriod(uint256 newGracePeriod)
external override onlyRole(DEFAULT_ADMIN_ROLE)
{
if (newGracePeriod < MINIMUM_GRACE_PERIOD) revert GracePeriodTooShort();
_updateGracePeriod(newGracePeriod);
}

/******************************************************************************************************************/
/*** External misc functions ***/
/******************************************************************************************************************/

/// @inheritdoc IExecutor
function executeDelegateCall(address target, bytes calldata data)
external
Expand All @@ -195,20 +203,9 @@ contract Executor is IExecutor, AccessControl {
/// @inheritdoc IExecutor
function receiveFunds() external payable override {}

/// @inheritdoc IExecutor
function getDelay() external view override returns (uint256) {
return _delay;
}

/// @inheritdoc IExecutor
function getGracePeriod() external view override returns (uint256) {
return _gracePeriod;
}

/// @inheritdoc IExecutor
function getActionsSetCount() external view override returns (uint256) {
return _actionsSetCounter;
}
/******************************************************************************************************************/
/*** External view functions ***/
/******************************************************************************************************************/

/// @inheritdoc IExecutor
function getActionsSetById(uint256 actionsSetId)
Expand All @@ -222,33 +219,19 @@ contract Executor is IExecutor, AccessControl {

/// @inheritdoc IExecutor
function getCurrentState(uint256 actionsSetId) public view override returns (ActionsSetState) {
if (_actionsSetCounter <= actionsSetId) revert InvalidActionsSetId();
ActionsSet storage actionsSet = _actionsSets[actionsSetId];
if (actionsSet.canceled) {
return ActionsSetState.Canceled;
} else if (actionsSet.executed) {
return ActionsSetState.Executed;
} else if (block.timestamp > actionsSet.executionTime + _gracePeriod) {
return ActionsSetState.Expired;
} else {
return ActionsSetState.Queued;
}
}
if (actionsSetCount <= actionsSetId) revert InvalidActionsSetId();

/// @inheritdoc IExecutor
function isActionQueued(bytes32 actionHash) public view override returns (bool) {
return _queuedActions[actionHash];
}
ActionsSet storage actionsSet =_actionsSets[actionsSetId];

function _updateDelay(uint256 delay) internal {
emit DelayUpdate(_delay, delay);
_delay = delay;
if (actionsSet.canceled) return ActionsSetState.Canceled;
else if (actionsSet.executed) return ActionsSetState.Executed;
else if (block.timestamp > actionsSet.executionTime + gracePeriod) return ActionsSetState.Expired;
else return ActionsSetState.Queued;
}

function _updateGracePeriod(uint256 gracePeriod) internal {
emit GracePeriodUpdate(_gracePeriod, gracePeriod);
_gracePeriod = gracePeriod;
}
/******************************************************************************************************************/
/*** Internal ActionSet helper functions ***/
/******************************************************************************************************************/

function _executeTransaction(
address target,
Expand All @@ -260,26 +243,18 @@ contract Executor is IExecutor, AccessControl {
) internal returns (bytes memory) {
if (address(this).balance < value) revert InsufficientBalance();

bytes32 actionHash = keccak256(
abi.encode(target, value, signature, data, executionTime, withDelegatecall)
);
_queuedActions[actionHash] = false;
_removeActionFromQueue(target, value, signature, data, executionTime, withDelegatecall);

bytes memory callData;
if (bytes(signature).length == 0) {
callData = data;
} else {
callData = abi.encodePacked(bytes4(keccak256(bytes(signature))), data);
}
bytes memory callData = bytes(signature).length == 0
? data
: abi.encodePacked(bytes4(keccak256(bytes(signature))), data);

if (withDelegatecall) {
return this.executeDelegateCall{value: value}(target, callData);
} else {
return target.functionCallWithValue(callData, value);
}
if (withDelegatecall) return this.executeDelegateCall{value: value}(target, callData);

return target.functionCallWithValue(callData, value);
}

function _cancelTransaction(
function _removeActionFromQueue(
address target,
uint256 value,
string memory signature,
Expand All @@ -290,7 +265,21 @@ contract Executor is IExecutor, AccessControl {
bytes32 actionHash = keccak256(
abi.encode(target, value, signature, data, executionTime, withDelegatecall)
);
_queuedActions[actionHash] = false;
isActionQueued[actionHash] = false;
}

/******************************************************************************************************************/
/*** Internal admin helper functions ***/
/******************************************************************************************************************/

function _updateDelay(uint256 newDelay) internal {
emit DelayUpdate(delay, newDelay);
delay = newDelay;
}

function _updateGracePeriod(uint256 newGracePeriod) internal {
emit GracePeriodUpdate(gracePeriod, newGracePeriod);
gracePeriod = newGracePeriod;
}

}
Loading

0 comments on commit 9374feb

Please sign in to comment.