Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

formatting: First round of cleanup #21

Merged
merged 8 commits into from
Jun 21, 2024
Merged

formatting: First round of cleanup #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
758da27
refactor to simplify into 1 contract and interface
hexonaut Jun 13, 2024
999d935
refactor guardian to be part of access control, also queue becomes a …
hexonaut Jun 13, 2024
2e7518d
formatting
hexonaut Jun 13, 2024
693d7e7
feat: update with new cleanup
lucas-manuel Jun 13, 2024
dcf7ed0
fix: conflicts
lucas-manuel Jun 13, 2024
01ed799
feat: add section labels, reorder
lucas-manuel Jun 13, 2024
2bb7f19
fix: conflicts
lucas-manuel Jun 14, 2024
5fd7162
chore: reduce diff
lucas-manuel Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
205 changes: 97 additions & 108 deletions src/Executor.sol
View file
Open in desktop
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
Loading