Skip to content

Commit

Permalink
Merge branch 'master' into SC-456-use-generic-receivers
Browse files Browse the repository at this point in the history
  • Loading branch information
@hexonaut
hexonaut committed Jun 11, 2024
2 parents 7fb9696 + 2fb0afc commit be2684b
Show file tree
Hide file tree
Showing 10 changed files with 10 additions and 168 deletions.
11 changes: 1 addition & 10 deletions src/executors/AuthBridgeExecutor.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,26 @@ import { BridgeExecutorBase } from './BridgeExecutorBase.sol';
* @notice Queue up proposals from an authorized bridge.
*/
contract AuthBridgeExecutor is IAuthBridgeExecutor, AccessControl, BridgeExecutorBase {

bytes32 public constant AUTHORIZED_BRIDGE_ROLE = keccak256('AUTHORIZED_BRIDGE_ROLE');

/**
* @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
* @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
)
{
_grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
_setRoleAdmin(AUTHORIZED_BRIDGE_ROLE, DEFAULT_ADMIN_ROLE);
}

/// @inheritdoc IAuthBridgeExecutor
Expand All @@ -49,7 +40,7 @@ contract AuthBridgeExecutor is IAuthBridgeExecutor, AccessControl, BridgeExecuto
string[] memory signatures,
bytes[] memory calldatas,
bool[] memory withDelegatecalls
) external onlyRole(AUTHORIZED_BRIDGE_ROLE) {
) external onlyRole(DEFAULT_ADMIN_ROLE) {
_queue(targets, values, signatures, calldatas, withDelegatecalls);
}

Expand Down
55 changes: 1 addition & 54 deletions src/executors/BridgeExecutorBase.sol
View file
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
View file
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
4 changes: 1 addition & 3 deletions test/ArbitrumOneCrosschainTest.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,13 @@ contract ArbitrumOneCrosschainTest is CrosschainTestBase {
bridgeExecutor = new AuthBridgeExecutor(
defaultL2BridgeExecutorArgs.delay,
defaultL2BridgeExecutorArgs.gracePeriod,
defaultL2BridgeExecutorArgs.minimumDelay,
defaultL2BridgeExecutorArgs.maximumDelay,
defaultL2BridgeExecutorArgs.guardian
);
bridgeReceiver = address(new ArbitrumReceiver(
defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor,
address(bridgeExecutor)
));
bridgeExecutor.grantRole(bridgeExecutor.AUTHORIZED_BRIDGE_ROLE(), bridgeReceiver);
bridgeExecutor.grantRole(bridgeExecutor.DEFAULT_ADMIN_ROLE(), bridgeReceiver);

bridge.source.selectFork();
vm.deal(L1_EXECUTOR, 0.01 ether);
Expand Down
18 changes: 4 additions & 14 deletions test/AuthBridgeExecutor.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,10 @@ 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);
executor.grantRole(executor.DEFAULT_ADMIN_ROLE(), bridge);
executor.revokeRole(executor.DEFAULT_ADMIN_ROLE(), address(this));
executor.grantRole(executor.DEFAULT_ADMIN_ROLE(), bridge);
executor.revokeRole(executor.DEFAULT_ADMIN_ROLE(), address(this));
}

/******************************************************************************************************************/
Expand Down Expand Up @@ -160,16 +157,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 +177,6 @@ contract AuthBridgeExecutorConstructorTests is AuthBridgeExecutorTestBase {
executor = new AuthBridgeExecutor({
delay: DELAY,
gracePeriod: GRACE_PERIOD,
minimumDelay: 0,
maximumDelay: 365 days,
guardian: guardian
});

Expand All @@ -194,15 +185,14 @@ contract AuthBridgeExecutorConstructorTests is AuthBridgeExecutorTestBase {
assertEq(executor.getGuardian(), guardian);

assertEq(executor.hasRole(executor.DEFAULT_ADMIN_ROLE(), address(this)), true);
assertEq(executor.getRoleAdmin(executor.AUTHORIZED_BRIDGE_ROLE()), executor.DEFAULT_ADMIN_ROLE());
}

}

contract AuthBridgeExecutorQueueTests is AuthBridgeExecutorTestBase {

function test_queue_onlyBridge() public {
vm.expectRevert(abi.encodeWithSignature("AccessControlUnauthorizedAccount(address,bytes32)", address(this), executor.AUTHORIZED_BRIDGE_ROLE()));
function test_queue_onlyDefaultAdmin() public {
vm.expectRevert(abi.encodeWithSignature("AccessControlUnauthorizedAccount(address,bytes32)", address(this), executor.DEFAULT_ADMIN_ROLE()));
executor.queue(new address[](0), new uint256[](0), new string[](0), new bytes[](0), new bool[](0));
}

Expand Down
4 changes: 1 addition & 3 deletions test/BaseChainCrosschainTest.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,13 @@ contract BaseChainCrosschainTest is CrosschainTestBase {
bridgeExecutor = new AuthBridgeExecutor(
defaultL2BridgeExecutorArgs.delay,
defaultL2BridgeExecutorArgs.gracePeriod,
defaultL2BridgeExecutorArgs.minimumDelay,
defaultL2BridgeExecutorArgs.maximumDelay,
defaultL2BridgeExecutorArgs.guardian
);
bridgeReceiver = address(new OptimismReceiver(
defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor,
address(bridgeExecutor)
));
bridgeExecutor.grantRole(bridgeExecutor.AUTHORIZED_BRIDGE_ROLE(), bridgeReceiver);
bridgeExecutor.grantRole(bridgeExecutor.DEFAULT_ADMIN_ROLE(), bridgeReceiver);

bridge.source.selectFork();
}
Expand Down
24 changes: 0 additions & 24 deletions test/CrosschainTestBase.sol
View file
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 @@ -75,8 +73,6 @@ abstract contract CrosschainTestBase is Test {
ethereumGovernanceExecutor: L1_EXECUTOR,
delay: 600,
gracePeriod: 1200,
minimumDelay: 0,
maximumDelay: 2400,
guardian: GUARDIAN
});

Expand Down Expand Up @@ -319,14 +315,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 @@ -336,16 +324,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 @@ -376,14 +360,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
4 changes: 1 addition & 3 deletions test/GnosisCrosschainTest.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ contract GnosisCrosschainTest is CrosschainTestBase {
bridgeExecutor = new AuthBridgeExecutor(
defaultL2BridgeExecutorArgs.delay,
defaultL2BridgeExecutorArgs.gracePeriod,
defaultL2BridgeExecutorArgs.minimumDelay,
defaultL2BridgeExecutorArgs.maximumDelay,
defaultL2BridgeExecutorArgs.guardian
);
bridgeReceiver = address(new AMBReceiver(
Expand All @@ -50,7 +48,7 @@ contract GnosisCrosschainTest is CrosschainTestBase {
defaultL2BridgeExecutorArgs.ethereumGovernanceExecutor,
address(bridgeExecutor)
));
bridgeExecutor.grantRole(bridgeExecutor.AUTHORIZED_BRIDGE_ROLE(), bridgeReceiver);
bridgeExecutor.grantRole(bridgeExecutor.DEFAULT_ADMIN_ROLE(), bridgeReceiver);

bridge.source.selectFork();
}
Expand Down
Loading

0 comments on commit be2684b

Please sign in to comment.