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.

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

[SC-429] Remove min and max delay #15

Merged
merged 1 commit into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 0 additions & 6 deletions src/executors/AuthBridgeExecutor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,16 @@ contract AuthBridgeExecutor is IAuthBridgeExecutor, AccessControl, BridgeExecuto
*
* @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
* @param minimumDelay The minimum bound a delay can be set to
* @param maximumDelay The maximum bound a delay can be set to
* @param guardian The address of the guardian, which can cancel queued proposals (can be zero)
*/
constructor(
uint256 delay,
uint256 gracePeriod,
uint256 minimumDelay,
uint256 maximumDelay,
address guardian
)
BridgeExecutorBase(
delay,
gracePeriod,
minimumDelay,
maximumDelay,
guardian
)
{
Expand Down
55 changes: 1 addition & 54 deletions src/executors/BridgeExecutorBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ abstract contract BridgeExecutorBase is IExecutorBase {
uint256 private _delay;
// Time after the execution time during which the actions set can be executed
uint256 private _gracePeriod;
// Minimum allowed delay
uint256 private _minimumDelay;
// Maximum allowed delay
uint256 private _maximumDelay;
// Address with the ability of canceling actions sets
address private _guardian;

Expand Down Expand Up @@ -53,28 +49,19 @@ abstract contract BridgeExecutorBase is IExecutorBase {
*
* @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
* @param minimumDelay The minimum bound a delay can be set to
* @param maximumDelay The maximum bound a delay can be set to
* @param guardian The address of the guardian, which can cancel queued proposals (can be zero)
*/
constructor(
uint256 delay,
uint256 gracePeriod,
uint256 minimumDelay,
uint256 maximumDelay,
address guardian
) {
if (
gracePeriod < MINIMUM_GRACE_PERIOD ||
minimumDelay >= maximumDelay ||
delay < minimumDelay ||
delay > maximumDelay
gracePeriod < MINIMUM_GRACE_PERIOD
) revert InvalidInitParams();

_updateDelay(delay);
_updateGracePeriod(gracePeriod);
_updateMinimumDelay(minimumDelay);
_updateMaximumDelay(maximumDelay);
_updateGuardian(guardian);
}

Expand Down Expand Up @@ -138,7 +125,6 @@ abstract contract BridgeExecutorBase is IExecutorBase {

/// @inheritdoc IExecutorBase
function updateDelay(uint256 delay) external override onlyThis {
_validateDelay(delay);
_updateDelay(delay);
}

Expand All @@ -148,20 +134,6 @@ abstract contract BridgeExecutorBase is IExecutorBase {
_updateGracePeriod(gracePeriod);
}

/// @inheritdoc IExecutorBase
function updateMinimumDelay(uint256 minimumDelay) external override onlyThis {
if (minimumDelay >= _maximumDelay) revert MinimumDelayTooLong();
_updateMinimumDelay(minimumDelay);
_validateDelay(_delay);
}

/// @inheritdoc IExecutorBase
function updateMaximumDelay(uint256 maximumDelay) external override onlyThis {
if (maximumDelay <= _minimumDelay) revert MaximumDelayTooShort();
_updateMaximumDelay(maximumDelay);
_validateDelay(_delay);
}

/// @inheritdoc IExecutorBase
function executeDelegateCall(address target, bytes calldata data)
external
Expand Down Expand Up @@ -190,16 +162,6 @@ abstract contract BridgeExecutorBase is IExecutorBase {
return _gracePeriod;
}

/// @inheritdoc IExecutorBase
function getMinimumDelay() external view override returns (uint256) {
return _minimumDelay;
}

/// @inheritdoc IExecutorBase
function getMaximumDelay() external view override returns (uint256) {
return _maximumDelay;
}

/// @inheritdoc IExecutorBase
function getGuardian() external view override returns (address) {
return _guardian;
Expand Down Expand Up @@ -255,16 +217,6 @@ abstract contract BridgeExecutorBase is IExecutorBase {
_gracePeriod = gracePeriod;
}

function _updateMinimumDelay(uint256 minimumDelay) internal {
emit MinimumDelayUpdate(_minimumDelay, minimumDelay);
_minimumDelay = minimumDelay;
}

function _updateMaximumDelay(uint256 maximumDelay) internal {
emit MaximumDelayUpdate(_maximumDelay, maximumDelay);
_maximumDelay = maximumDelay;
}

/**
* @notice Queue an ActionsSet
* @dev If a signature is empty, calldata is used for the execution, calldata is appended to signature otherwise
Expand Down Expand Up @@ -380,11 +332,6 @@ abstract contract BridgeExecutorBase is IExecutorBase {
_queuedActions[actionHash] = false;
}

function _validateDelay(uint256 delay) internal view {
if (delay < _minimumDelay) revert DelayShorterThanMin();
if (delay > _maximumDelay) revert DelayLongerThanMax();
}

function _verifyCallResult(bool success, bytes memory returnData)
private pure returns (bytes memory)
{
Expand Down
38 changes: 0 additions & 38 deletions src/interfaces/IExecutorBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -115,20 +115,6 @@ interface IExecutorBase {
**/
event GracePeriodUpdate(uint256 oldGracePeriod, uint256 newGracePeriod);

/**
* @dev Emitted when the minimum delay (lower bound of delay) is updated
* @param oldMinimumDelay The value of the old minimum delay
* @param newMinimumDelay The value of the new minimum delay
**/
event MinimumDelayUpdate(uint256 oldMinimumDelay, uint256 newMinimumDelay);

/**
* @dev Emitted when the maximum delay (upper bound of delay)is updated
* @param oldMaximumDelay The value of the old maximum delay
* @param newMaximumDelay The value of the new maximum delay
**/
event MaximumDelayUpdate(uint256 oldMaximumDelay, uint256 newMaximumDelay);

/**
* @notice Execute the ActionsSet
* @param actionsSetId The id of the ActionsSet to execute
Expand Down Expand Up @@ -160,18 +146,6 @@ interface IExecutorBase {
**/
function updateGracePeriod(uint256 gracePeriod) external;

/**
* @notice Update the minimum allowed delay
* @param minimumDelay The value of the minimum delay (in seconds)
**/
function updateMinimumDelay(uint256 minimumDelay) external;

/**
* @notice Update the maximum allowed delay
* @param maximumDelay The maximum delay (in seconds)
**/
function updateMaximumDelay(uint256 maximumDelay) external;

/**
* @notice Allows to delegatecall a given target with an specific amount of value
* @dev This function is external so it allows to specify a defined msg.value for the delegate call, reducing
Expand Down Expand Up @@ -202,18 +176,6 @@ interface IExecutorBase {
**/
function getGracePeriod() external view returns (uint256);

/**
* @notice Returns the minimum delay
* @return The value of the minimum delay (in seconds)
**/
function getMinimumDelay() external view returns (uint256);

/**
* @notice Returns the maximum delay
* @return The value of the maximum delay (in seconds)
**/
function getMaximumDelay() external view returns (uint256);

/**
* @notice Returns the address of the guardian
* @return The address of the guardian
Expand Down
2 changes: 0 additions & 2 deletions test/ArbitrumOneCrosschainTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ contract ArbitrumOneCrosschainTest is CrosschainTestBase {
bridgeExecutor = new AuthBridgeExecutor(
defaultL2BridgeExecutorArgs.delay,
defaultL2BridgeExecutorArgs.gracePeriod,
defaultL2BridgeExecutorArgs.minimumDelay,
defaultL2BridgeExecutorArgs.maximumDelay,
defaultL2BridgeExecutorArgs.guardian
);
bridgeReceiver = address(new BridgeExecutorReceiverArbitrum(
Expand Down
8 changes: 0 additions & 8 deletions test/AuthBridgeExecutor.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ contract AuthBridgeExecutorTestBase is Test {
executor = new AuthBridgeExecutor({
delay: DELAY,
gracePeriod: GRACE_PERIOD,
minimumDelay: 0, // TODO: removing this in next PR
maximumDelay: 365 days, // TODO: removing this in next PR
guardian: guardian
});
executor.grantRole(executor.AUTHORIZED_BRIDGE_ROLE(), bridge);
Expand Down Expand Up @@ -160,16 +158,12 @@ contract AuthBridgeExecutorConstructorTests is AuthBridgeExecutorTestBase {
executor = new AuthBridgeExecutor({
delay: DELAY,
gracePeriod: 10 minutes - 1,
minimumDelay: 0,
maximumDelay: 365 days,
guardian: guardian
});

executor = new AuthBridgeExecutor({
delay: DELAY,
gracePeriod: 10 minutes,
minimumDelay: 0,
maximumDelay: 365 days,
guardian: guardian
});
}
Expand All @@ -184,8 +178,6 @@ contract AuthBridgeExecutorConstructorTests is AuthBridgeExecutorTestBase {
executor = new AuthBridgeExecutor({
delay: DELAY,
gracePeriod: GRACE_PERIOD,
minimumDelay: 0,
maximumDelay: 365 days,
guardian: guardian
});

Expand Down
2 changes: 0 additions & 2 deletions test/BaseChainCrosschainTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ contract BaseChainCrosschainTest is CrosschainTestBase {
bridgeExecutor = new AuthBridgeExecutor(
defaultL2BridgeExecutorArgs.delay,
defaultL2BridgeExecutorArgs.gracePeriod,
defaultL2BridgeExecutorArgs.minimumDelay,
defaultL2BridgeExecutorArgs.maximumDelay,
defaultL2BridgeExecutorArgs.guardian
);
bridgeReceiver = address(new BridgeExecutorReceiverOptimism(
Expand Down
24 changes: 0 additions & 24 deletions test/CrosschainTestBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ struct L2BridgeExecutorArguments {
address ethereumGovernanceExecutor;
uint256 delay;
uint256 gracePeriod;
uint256 minimumDelay;
uint256 maximumDelay;
address guardian;
}

Expand Down Expand Up @@ -72,8 +70,6 @@ abstract contract CrosschainTestBase is Test {
ethereumGovernanceExecutor: L1_EXECUTOR,
delay: 600,
gracePeriod: 1200,
minimumDelay: 0,
maximumDelay: 2400,
guardian: GUARDIAN
});

Expand Down Expand Up @@ -316,14 +312,6 @@ abstract contract CrosschainTestBase is Test {
bridgeExecutor.getGracePeriod(),
defaultL2BridgeExecutorArgs.gracePeriod
);
assertEq(
bridgeExecutor.getMinimumDelay(),
defaultL2BridgeExecutorArgs.minimumDelay
);
assertEq(
bridgeExecutor.getMaximumDelay(),
defaultL2BridgeExecutorArgs.maximumDelay
);
assertEq(
bridgeExecutor.getGuardian(),
defaultL2BridgeExecutorArgs.guardian
Expand All @@ -333,16 +321,12 @@ abstract contract CrosschainTestBase is Test {
ethereumGovernanceExecutor: defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor,
delay: 1200,
gracePeriod: 1800,
minimumDelay: 100,
maximumDelay: 3600,
guardian: makeAddr('newGuardian')
});

IPayload reconfigurationPayload = IPayload(new ReconfigurationPayload(
newL2BridgeExecutorParams.delay,
newL2BridgeExecutorParams.gracePeriod,
newL2BridgeExecutorParams.minimumDelay,
newL2BridgeExecutorParams.maximumDelay,
newL2BridgeExecutorParams.guardian
));

Expand Down Expand Up @@ -373,14 +357,6 @@ abstract contract CrosschainTestBase is Test {
bridgeExecutor.getGracePeriod(),
newL2BridgeExecutorParams.gracePeriod
);
assertEq(
bridgeExecutor.getMinimumDelay(),
newL2BridgeExecutorParams.minimumDelay
);
assertEq(
bridgeExecutor.getMaximumDelay(),
newL2BridgeExecutorParams.maximumDelay
);
assertEq(
bridgeExecutor.getGuardian(),
newL2BridgeExecutorParams.guardian
Expand Down
2 changes: 0 additions & 2 deletions test/GnosisCrosschainTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ contract GnosisCrosschainTest is CrosschainTestBase {
bridgeExecutor = new AuthBridgeExecutor(
defaultL2BridgeExecutorArgs.delay,
defaultL2BridgeExecutorArgs.gracePeriod,
defaultL2BridgeExecutorArgs.minimumDelay,
defaultL2BridgeExecutorArgs.maximumDelay,
defaultL2BridgeExecutorArgs.guardian
);
bridgeReceiver = address(new BridgeExecutorReceiverGnosis(
Expand Down
2 changes: 0 additions & 2 deletions test/OptimismCrosschainTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ contract OptimismCrosschainTest is CrosschainTestBase {
bridgeExecutor = new AuthBridgeExecutor(
defaultL2BridgeExecutorArgs.delay,
defaultL2BridgeExecutorArgs.gracePeriod,
defaultL2BridgeExecutorArgs.minimumDelay,
defaultL2BridgeExecutorArgs.maximumDelay,
defaultL2BridgeExecutorArgs.guardian
);
bridgeReceiver = address(new BridgeExecutorReceiverOptimism(
Expand Down
16 changes: 0 additions & 16 deletions test/mocks/ReconfigurationPayload.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,21 @@ contract ReconfigurationPayload is IPayload {

uint256 public immutable newDelay;
uint256 public immutable newGracePeriod;
uint256 public immutable newMinimumDelay;
uint256 public immutable newMaximumDelay;
address public immutable newGuardian;

constructor(
uint256 _newDelay,
uint256 _newGracePeriod,
uint256 _newMinimumDelay,
uint256 _newMaximumDelay,
address _newGuardian
) {
newDelay = _newDelay;
newGracePeriod = _newGracePeriod;
newMinimumDelay = _newMinimumDelay;
newMaximumDelay = _newMaximumDelay;
newGuardian = _newGuardian;
}

function execute() external override {
IExecutorBase(address(this)).updateDelay(getNewDelay());
IExecutorBase(address(this)).updateGracePeriod(getNewGracePeriod());
IExecutorBase(address(this)).updateMinimumDelay(getNewMinimumDelay());
IExecutorBase(address(this)).updateMaximumDelay(getNewMaximumDelay());
IExecutorBase(address(this)).updateGuardian(getNewGuardian());
}

Expand All @@ -46,14 +38,6 @@ contract ReconfigurationPayload is IPayload {
return newGracePeriod;
}

function getNewMinimumDelay() public view returns (uint256) {
return newMinimumDelay;
}

function getNewMaximumDelay() public view returns (uint256) {
return newMaximumDelay;
}

function getNewGuardian() public view returns (address) {
return newGuardian;
}
Expand Down
Loading