From ac2d51143b76d37e19059270e126985156bc754d Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Thu, 24 Feb 2022 10:52:51 -0500 Subject: [PATCH 1/4] WIP Signed-off-by: Matt Rice --- contracts/Arbitrum_SpokePool.sol | 2 ++ contracts/Ethereum_SpokePool.sol | 2 ++ contracts/Optimism_SpokePool.sol | 55 ++++---------------------------- contracts/SpokePool.sol | 43 ++++++++++++++----------- 4 files changed, 36 insertions(+), 66 deletions(-) diff --git a/contracts/Arbitrum_SpokePool.sol b/contracts/Arbitrum_SpokePool.sol index 4da141f25..d40cfb9ee 100644 --- a/contracts/Arbitrum_SpokePool.sol +++ b/contracts/Arbitrum_SpokePool.sol @@ -116,4 +116,6 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool { l2Address = address(uint160(l1Address) + uint160(0x1111000000000000000000000000000000001111)); } } + + function _requireAdminSender() internal override onlyFromCrossDomainAdmin {} } diff --git a/contracts/Ethereum_SpokePool.sol b/contracts/Ethereum_SpokePool.sol index aea647277..528370f33 100644 --- a/contracts/Ethereum_SpokePool.sol +++ b/contracts/Ethereum_SpokePool.sol @@ -55,4 +55,6 @@ contract Ethereum_SpokePool is SpokePoolInterface, SpokePool, Ownable { function _bridgeTokensToHubPool(RelayerRefundLeaf memory relayerRefundLeaf) internal override { IERC20(relayerRefundLeaf.l2TokenAddress).transfer(hubPool, relayerRefundLeaf.amountToReturn); } + + function _requireAdminSender() internal override onlyOwner {} } diff --git a/contracts/Optimism_SpokePool.sol b/contracts/Optimism_SpokePool.sol index 8fa1049ca..79f19eca2 100644 --- a/contracts/Optimism_SpokePool.sol +++ b/contracts/Optimism_SpokePool.sol @@ -32,62 +32,19 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool SpokePool(_crossDomainAdmin, _hubPool, 0x4200000000000000000000000000000000000006, timerAddress) {} - /************************************** - * CROSS-CHAIN ADMIN FUNCTIONS * - **************************************/ + /******************************************* + * OPTIMISM SPECIFIC ADMIN FUNCTIONS * + *******************************************/ function setL1GasLimit(uint32 newl1Gas) public onlyFromCrossDomainAccount(crossDomainAdmin) { - _setL1GasLimit(newl1Gas); - } - - function setCrossDomainAdmin(address newCrossDomainAdmin) - public - override - onlyFromCrossDomainAccount(crossDomainAdmin) - nonReentrant - { - _setCrossDomainAdmin(newCrossDomainAdmin); - } - - function setHubPool(address newHubPool) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { - _setHubPool(newHubPool); - } - - function setEnableRoute( - address originToken, - uint256 destinationChainId, - bool enable - ) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { - _setEnableRoute(originToken, destinationChainId, enable); - } - - function setDepositQuoteTimeBuffer(uint32 buffer) - public - override - onlyFromCrossDomainAccount(crossDomainAdmin) - nonReentrant - { - _setDepositQuoteTimeBuffer(buffer); - } - - function relayRootBundle(bytes32 relayerRefundRoot, bytes32 slowRelayRoot) - public - override - onlyFromCrossDomainAccount(crossDomainAdmin) - nonReentrant - { - _relayRootBundle(relayerRefundRoot, slowRelayRoot); + l1Gas = newl1Gas; + emit SetL1Gas(newl1Gas); } /************************************** * INTERNAL FUNCTIONS * **************************************/ - function _setL1GasLimit(uint32 _l1Gas) internal { - l1Gas = _l1Gas; - emit SetL1Gas(l1Gas); - } - function _bridgeTokensToHubPool(RelayerRefundLeaf memory relayerRefundLeaf) internal override { // If the token being bridged is WETH then we need to first unwrap it to ETH and then send ETH over the // canonical bridge. On Optimism, this is address 0xDeadDeAddeAddEAddeadDEaDDEAdDeaDDeAD0000. @@ -105,4 +62,6 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool emit OptimismTokensBridged(relayerRefundLeaf.l2TokenAddress, hubPool, relayerRefundLeaf.amountToReturn, l1Gas); } + + function _requireAdminSender() internal override onlyFromCrossDomainAccount(crossDomainAdmin) {} } diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index c866cd8bb..72e67a833 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -158,32 +158,50 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall * ADMIN FUNCTIONS * **************************************/ - function _setCrossDomainAdmin(address newCrossDomainAdmin) internal { + function setCrossDomainAdmin(address newCrossDomainAdmin) public override nonReentrant { + _requireAdminSender(); require(newCrossDomainAdmin != address(0), "Bad bridge router address"); crossDomainAdmin = newCrossDomainAdmin; emit SetXDomainAdmin(crossDomainAdmin); } - function _setHubPool(address newHubPool) internal { + function setHubPool(address newHubPool) public override nonReentrant { + _requireAdminSender(); require(newHubPool != address(0), "Bad hub pool address"); hubPool = newHubPool; emit SetHubPool(hubPool); } - function _setEnableRoute( + function setEnableRoute( address originToken, uint256 destinationChainId, bool enabled - ) internal { + ) public override nonReentrant { + _requireAdminSender(); enabledDepositRoutes[originToken][destinationChainId] = enabled; emit EnabledDepositRoute(originToken, destinationChainId, enabled); } - function _setDepositQuoteTimeBuffer(uint32 _depositQuoteTimeBuffer) internal { + function setDepositQuoteTimeBuffer(uint32 _depositQuoteTimeBuffer) public override nonReentrant { + _requireAdminSender(); depositQuoteTimeBuffer = _depositQuoteTimeBuffer; emit SetDepositQuoteTimeBuffer(_depositQuoteTimeBuffer); } + // This internal method should be called by an external "relayRootBundle" function that validates the + // cross domain sender is the HubPool. This validation step differs for each L2, which is why the implementation + // specifics are left to the implementor of this abstract contract. + // Once this method is executed and a distribution root is stored in this contract, then `distributeRootBundle` + // can be called to execute each leaf in the root. + function relayRootBundle(bytes32 relayerRefundRoot, bytes32 slowRelayRoot) public override nonReentrant { + _requireAdminSender(); + uint32 rootBundleId = uint32(rootBundles.length); + RootBundle storage rootBundle = rootBundles.push(); + rootBundle.relayerRefundRoot = relayerRefundRoot; + rootBundle.slowRelayRoot = slowRelayRoot; + emit RelayedRootBundle(rootBundleId, relayerRefundRoot, slowRelayRoot); + } + /************************************** * DEPOSITOR FUNCTIONS * **************************************/ @@ -476,19 +494,6 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall } } - // This internal method should be called by an external "relayRootBundle" function that validates the - // cross domain sender is the HubPool. This validation step differs for each L2, which is why the implementation - // specifics are left to the implementor of this abstract contract. - // Once this method is executed and a distribution root is stored in this contract, then `distributeRootBundle` - // can be called to execute each leaf in the root. - function _relayRootBundle(bytes32 relayerRefundRoot, bytes32 slowRelayRoot) internal { - uint32 rootBundleId = uint32(rootBundles.length); - RootBundle storage rootBundle = rootBundles.push(); - rootBundle.relayerRefundRoot = relayerRefundRoot; - rootBundle.slowRelayRoot = slowRelayRoot; - emit RelayedRootBundle(rootBundleId, relayerRefundRoot, slowRelayRoot); - } - function _fillRelay( bytes32 relayHash, RelayData memory relayData, @@ -586,6 +591,8 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall ); } + function _requireAdminSender() internal virtual; + // Added to enable the this contract to receive ETH. Used when unwrapping Weth. receive() external payable {} } From 76a464e16a0b933ea1017bb177cccb7fc7df1fd8 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Fri, 25 Feb 2022 12:31:50 -0500 Subject: [PATCH 2/4] WIP Signed-off-by: Matt Rice --- contracts/Arbitrum_SpokePool.sol | 39 ++++---------------------------- contracts/Ethereum_SpokePool.sol | 26 +-------------------- contracts/Optimism_SpokePool.sol | 4 ++-- contracts/SpokePool.sol | 37 +++++++++++++++++++----------- 4 files changed, 32 insertions(+), 74 deletions(-) diff --git a/contracts/Arbitrum_SpokePool.sol b/contracts/Arbitrum_SpokePool.sol index d40cfb9ee..b9ba6f258 100644 --- a/contracts/Arbitrum_SpokePool.sol +++ b/contracts/Arbitrum_SpokePool.sol @@ -45,47 +45,18 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool { _; } - /************************************** - * CROSS-CHAIN ADMIN FUNCTIONS * - **************************************/ + /******************************************************** + * ARBITRUM-SPECIFIC CROSS-CHAIN ADMIN FUNCTIONS * + ********************************************************/ - function setL2GatewayRouter(address newL2GatewayRouter) public onlyFromCrossDomainAdmin nonReentrant { + function setL2GatewayRouter(address newL2GatewayRouter) public onlyAdmin nonReentrant { _setL2GatewayRouter(newL2GatewayRouter); } - function whitelistToken(address l2Token, address l1Token) public onlyFromCrossDomainAdmin nonReentrant { + function whitelistToken(address l2Token, address l1Token) public onlyAdmin nonReentrant { _whitelistToken(l2Token, l1Token); } - function setCrossDomainAdmin(address newCrossDomainAdmin) public override onlyFromCrossDomainAdmin nonReentrant { - _setCrossDomainAdmin(newCrossDomainAdmin); - } - - function setHubPool(address newHubPool) public override onlyFromCrossDomainAdmin nonReentrant { - _setHubPool(newHubPool); - } - - function setEnableRoute( - address originToken, - uint256 destinationChainId, - bool enable - ) public override onlyFromCrossDomainAdmin nonReentrant { - _setEnableRoute(originToken, destinationChainId, enable); - } - - function setDepositQuoteTimeBuffer(uint32 buffer) public override onlyFromCrossDomainAdmin nonReentrant { - _setDepositQuoteTimeBuffer(buffer); - } - - function relayRootBundle(bytes32 relayerRefundRoot, bytes32 slowRelayRoot) - public - override - onlyFromCrossDomainAdmin - nonReentrant - { - _relayRootBundle(relayerRefundRoot, slowRelayRoot); - } - /************************************** * INTERNAL FUNCTIONS * **************************************/ diff --git a/contracts/Ethereum_SpokePool.sol b/contracts/Ethereum_SpokePool.sol index 528370f33..a56761309 100644 --- a/contracts/Ethereum_SpokePool.sol +++ b/contracts/Ethereum_SpokePool.sol @@ -25,33 +25,9 @@ contract Ethereum_SpokePool is SpokePoolInterface, SpokePool, Ownable { ) SpokePool(_crossDomainAdmin, _hubPool, _wethAddress, timerAddress) {} /************************************** - * ADMIN FUNCTIONS * + * INTERNAL FUNCTIONS * **************************************/ - function setCrossDomainAdmin(address newCrossDomainAdmin) public override onlyOwner nonReentrant { - _setCrossDomainAdmin(newCrossDomainAdmin); - } - - function setHubPool(address newHubPool) public override onlyOwner nonReentrant { - _setHubPool(newHubPool); - } - - function setEnableRoute( - address originToken, - uint256 destinationChainId, - bool enable - ) public override onlyOwner nonReentrant { - _setEnableRoute(originToken, destinationChainId, enable); - } - - function setDepositQuoteTimeBuffer(uint32 buffer) public override onlyOwner nonReentrant { - _setDepositQuoteTimeBuffer(buffer); - } - - function relayRootBundle(bytes32 relayerRefundRoot, bytes32 slowRelayRoot) public override onlyOwner nonReentrant { - _relayRootBundle(relayerRefundRoot, slowRelayRoot); - } - function _bridgeTokensToHubPool(RelayerRefundLeaf memory relayerRefundLeaf) internal override { IERC20(relayerRefundLeaf.l2TokenAddress).transfer(hubPool, relayerRefundLeaf.amountToReturn); } diff --git a/contracts/Optimism_SpokePool.sol b/contracts/Optimism_SpokePool.sol index 79f19eca2..8cc0baf5d 100644 --- a/contracts/Optimism_SpokePool.sol +++ b/contracts/Optimism_SpokePool.sol @@ -33,10 +33,10 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool {} /******************************************* - * OPTIMISM SPECIFIC ADMIN FUNCTIONS * + * OPTIMISM-SPECIFIC ADMIN FUNCTIONS * *******************************************/ - function setL1GasLimit(uint32 newl1Gas) public onlyFromCrossDomainAccount(crossDomainAdmin) { + function setL1GasLimit(uint32 newl1Gas) public onlyAdmin { l1Gas = newl1Gas; emit SetL1Gas(newl1Gas); } diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index 72e67a833..26edaf3fe 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -154,35 +154,34 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall _; } + modifier onlyAdmin() { + _requireAdminSender(); + _; + } + /************************************** * ADMIN FUNCTIONS * **************************************/ - function setCrossDomainAdmin(address newCrossDomainAdmin) public override nonReentrant { - _requireAdminSender(); - require(newCrossDomainAdmin != address(0), "Bad bridge router address"); - crossDomainAdmin = newCrossDomainAdmin; - emit SetXDomainAdmin(crossDomainAdmin); + function setCrossDomainAdmin(address newCrossDomainAdmin) public override onlyAdmin nonReentrant { + _setCrossDomainAdmin(newCrossDomainAdmin); } - function setHubPool(address newHubPool) public override nonReentrant { - _requireAdminSender(); - require(newHubPool != address(0), "Bad hub pool address"); - hubPool = newHubPool; - emit SetHubPool(hubPool); + function setHubPool(address newHubPool) public override onlyAdmin nonReentrant { + _setHubPool(newHubPool); } function setEnableRoute( address originToken, uint256 destinationChainId, bool enabled - ) public override nonReentrant { + ) public override onlyAdmin nonReentrant { _requireAdminSender(); enabledDepositRoutes[originToken][destinationChainId] = enabled; emit EnabledDepositRoute(originToken, destinationChainId, enabled); } - function setDepositQuoteTimeBuffer(uint32 _depositQuoteTimeBuffer) public override nonReentrant { + function setDepositQuoteTimeBuffer(uint32 _depositQuoteTimeBuffer) public override onlyAdmin nonReentrant { _requireAdminSender(); depositQuoteTimeBuffer = _depositQuoteTimeBuffer; emit SetDepositQuoteTimeBuffer(_depositQuoteTimeBuffer); @@ -193,7 +192,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall // specifics are left to the implementor of this abstract contract. // Once this method is executed and a distribution root is stored in this contract, then `distributeRootBundle` // can be called to execute each leaf in the root. - function relayRootBundle(bytes32 relayerRefundRoot, bytes32 slowRelayRoot) public override nonReentrant { + function relayRootBundle(bytes32 relayerRefundRoot, bytes32 slowRelayRoot) public override onlyAdmin nonReentrant { _requireAdminSender(); uint32 rootBundleId = uint32(rootBundles.length); RootBundle storage rootBundle = rootBundles.push(); @@ -452,6 +451,18 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall * INTERNAL FUNCTIONS * **************************************/ + function _setCrossDomainAdmin(address newCrossDomainAdmin) internal { + require(newCrossDomainAdmin != address(0), "Bad bridge router address"); + crossDomainAdmin = newCrossDomainAdmin; + emit SetXDomainAdmin(crossDomainAdmin); + } + + function _setHubPool(address newHubPool) internal { + require(newHubPool != address(0), "Bad hub pool address"); + hubPool = newHubPool; + emit SetHubPool(hubPool); + } + function _bridgeTokensToHubPool(SpokePoolInterface.RelayerRefundLeaf memory relayerRefundLeaf) internal virtual; // Allow L2 to implement chain specific recovering of signers from signatures because some L2s might not support From d513e1bb35e0555ea0da906cadde15223d5712dd Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Fri, 25 Feb 2022 12:38:12 -0500 Subject: [PATCH 3/4] WIP Signed-off-by: Matt Rice --- contracts/test/MockSpokePool.sol | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/contracts/test/MockSpokePool.sol b/contracts/test/MockSpokePool.sol index 013ab87fa..2c8984612 100644 --- a/contracts/test/MockSpokePool.sol +++ b/contracts/test/MockSpokePool.sol @@ -16,29 +16,7 @@ contract MockSpokePool is SpokePoolInterface, SpokePool { address timerAddress ) SpokePool(_crossDomainAdmin, _hubPool, _wethAddress, timerAddress) {} - function setCrossDomainAdmin(address newCrossDomainAdmin) public override { - _setCrossDomainAdmin(newCrossDomainAdmin); - } - - function setHubPool(address newHubPool) public override { - _setHubPool(newHubPool); - } - - function setEnableRoute( - address originToken, - uint256 destinationChainId, - bool enable - ) public override { - _setEnableRoute(originToken, destinationChainId, enable); - } - - function setDepositQuoteTimeBuffer(uint32 buffer) public override { - _setDepositQuoteTimeBuffer(buffer); - } - - function relayRootBundle(bytes32 relayerRefundRoot, bytes32 slowRelayRoot) public override { - _relayRootBundle(relayerRefundRoot, slowRelayRoot); - } - function _bridgeTokensToHubPool(RelayerRefundLeaf memory relayerRefundLeaf) internal override {} + + function _requireAdminSender() internal override {} } From cbc8467296f817cd471245a87890d7f499d66525 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Fri, 25 Feb 2022 17:54:47 -0500 Subject: [PATCH 4/4] WIP Signed-off-by: Matt Rice --- contracts/SpokePool.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index 26edaf3fe..5952d73ce 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -176,13 +176,11 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall uint256 destinationChainId, bool enabled ) public override onlyAdmin nonReentrant { - _requireAdminSender(); enabledDepositRoutes[originToken][destinationChainId] = enabled; emit EnabledDepositRoute(originToken, destinationChainId, enabled); } function setDepositQuoteTimeBuffer(uint32 _depositQuoteTimeBuffer) public override onlyAdmin nonReentrant { - _requireAdminSender(); depositQuoteTimeBuffer = _depositQuoteTimeBuffer; emit SetDepositQuoteTimeBuffer(_depositQuoteTimeBuffer); } @@ -193,7 +191,6 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall // Once this method is executed and a distribution root is stored in this contract, then `distributeRootBundle` // can be called to execute each leaf in the root. function relayRootBundle(bytes32 relayerRefundRoot, bytes32 slowRelayRoot) public override onlyAdmin nonReentrant { - _requireAdminSender(); uint32 rootBundleId = uint32(rootBundles.length); RootBundle storage rootBundle = rootBundles.push(); rootBundle.relayerRefundRoot = relayerRefundRoot;