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

Spearbit Audit Changes #25

Merged
merged 5 commits into from
Aug 22, 2024
Merged

Spearbit Audit Changes #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3021d27
admin is already default admin role by default
hexonaut Aug 21, 2024
b0586cf
merge dupe grace period check
hexonaut Aug 21, 2024
1940e6c
spacing
hexonaut Aug 21, 2024
2ca228f
remove unused param
hexonaut Aug 21, 2024
94a84a5
better wording on comment
hexonaut Aug 21, 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
13 changes: 2 additions & 11 deletions src/Executor.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,9 @@ contract Executor is IExecutor, AccessControl {
uint256 delay_,
uint256 gracePeriod_
) {
if (
gracePeriod_ < MINIMUM_GRACE_PERIOD
) revert InvalidInitParams();

_updateDelay(delay_);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original comment here

_updateGracePeriod(gracePeriod_);

_setRoleAdmin(SUBMISSION_ROLE, DEFAULT_ADMIN_ROLE);
_setRoleAdmin(GUARDIAN_ROLE, DEFAULT_ADMIN_ROLE);

_grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
_grantRole(DEFAULT_ADMIN_ROLE, address(this)); // Necessary for self-referential calls to change configuration
}
Expand Down Expand Up @@ -120,7 +113,6 @@ contract Executor is IExecutor, AccessControl {
actionsSet.values[i],
actionsSet.signatures[i],
actionsSet.calldatas[i],
actionsSet.executionTime,
actionsSet.withDelegatecalls[i]
);
}
Expand Down Expand Up @@ -151,7 +143,6 @@ contract Executor is IExecutor, AccessControl {
function updateGracePeriod(uint256 newGracePeriod)
external override onlyRole(DEFAULT_ADMIN_ROLE)
{
if (newGracePeriod < MINIMUM_GRACE_PERIOD) revert GracePeriodTooShort();
_updateGracePeriod(newGracePeriod);
}

Expand Down Expand Up @@ -191,7 +182,7 @@ contract Executor is IExecutor, AccessControl {
function getCurrentState(uint256 actionsSetId) public view override returns (ActionsSetState) {
if (actionsSetCount <= actionsSetId) revert InvalidActionsSetId();

ActionsSet storage actionsSet =_actionsSets[actionsSetId];
ActionsSet storage actionsSet = _actionsSets[actionsSetId];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original comment here


if (actionsSet.canceled) return ActionsSetState.Canceled;
else if (actionsSet.executed) return ActionsSetState.Executed;
Expand All @@ -208,7 +199,6 @@ contract Executor is IExecutor, AccessControl {
uint256 value,
string memory signature,
bytes memory data,
uint256 executionTime,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original comment here

bool withDelegatecall
) internal returns (bytes memory) {
if (address(this).balance < value) revert InsufficientBalance();
Expand All @@ -232,6 +222,7 @@ contract Executor is IExecutor, AccessControl {
}

function _updateGracePeriod(uint256 newGracePeriod) internal {
if (newGracePeriod < MINIMUM_GRACE_PERIOD) revert GracePeriodTooShort();
emit GracePeriodUpdate(gracePeriod, newGracePeriod);
gracePeriod = newGracePeriod;
}
Expand Down
3 changes: 1 addition & 2 deletions src/interfaces/IExecutor.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ interface IExecutor is IAccessControl {
/*** Errors ***/
/******************************************************************************************************************/

error InvalidInitParams();
error GracePeriodTooShort();
error OnlyQueuedActions();
error TimelockNotFinished();
Expand Down Expand Up @@ -161,7 +160,7 @@ interface IExecutor is IAccessControl {
* @dev If a signature is empty, calldata is used for the execution, calldata is appended to signature otherwise.
* @param targets Array of targets to be called by the actions set.
* @param values Array of values to pass in each call by the actions set.
* @param signatures Array of function signatures to encode in each call by the actions (can be empty).
* @param signatures Array of function signatures to encode in each call by the actions which can be empty strings.
* @param calldatas Array of calldata to pass in each call by the actions set.
* @param withDelegatecalls Array of whether to delegatecall for each call of the actions set.
**/
Expand Down
2 changes: 1 addition & 1 deletion test/Executor.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ contract ExecutorTestBase is Test {
contract ExecutorConstructorTests is ExecutorTestBase {

function test_constructor_invalidInitParams_boundary() public {
vm.expectRevert(abi.encodeWithSignature("InvalidInitParams()"));
vm.expectRevert(abi.encodeWithSignature("GracePeriodTooShort()"));
executor = new Executor({
delay_: DELAY,
gracePeriod_: 10 minutes - 1
Expand Down
Loading