feat(protocol): improve _authorizePause for Bridge (#16544)

taikoxyz · Mar 28, 2024 · f76c705 · f76c705
1 parent c879124
commit f76c705
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 16 deletions.
diff --git a/packages/protocol/contracts/L1/TaikoL1.sol b/packages/protocol/contracts/L1/TaikoL1.sol
@@ -117,7 +117,7 @@ contract TaikoL1 is EssentialContract, ITaikoL1, TaikoEvents, TaikoErrors {
     /// @notice Pause block proving.
     /// @param _pause True if paused.
     function pauseProving(bool _pause) external {
-        _authorizePause(msg.sender);
+        _authorizePause(msg.sender, _pause);
         LibProving.pauseProving(state, _pause);
     }
 
@@ -216,7 +216,11 @@ contract TaikoL1 is EssentialContract, ITaikoL1, TaikoEvents, TaikoErrors {
         });
     }
 
-    function _authorizePause(address)
+    /// @dev chain_pauser is supposed to be a cold wallet.
+    function _authorizePause(
+        address,
+        bool
+    )
         internal
         view
         virtual

diff --git a/packages/protocol/contracts/L2/DelegateOwner.sol b/packages/protocol/contracts/L2/DelegateOwner.sol
@@ -88,7 +88,7 @@ contract DelegateOwner is EssentialContract, IMessageInvocable {
         emit OwnershipAccepted(target);
     }
 
-    function _authorizePause(address) internal pure override {
+    function _authorizePause(address, bool) internal pure override {
         revert DO_UNSUPPORTED();
     }
 }
diff --git a/packages/protocol/contracts/bridge/Bridge.sol b/packages/protocol/contracts/bridge/Bridge.sol
@@ -468,14 +468,18 @@ contract Bridge is EssentialContract, IBridge {
         return _msgHash ^ bytes32(uint256(Status.FAILED));
     }
 
-    /// @notice Checks if the given address can pause and unpause the bridge.
-    function _authorizePause(address)
-        internal
-        view
-        virtual
-        override
-        onlyFromOwnerOrNamed("bridge_pauser")
-    { }
+    /// @notice Checks if the given address can pause and/or unpause the bridge.
+    /// @dev Considering that the watchdog is a hot wallet, in case its private key is leaked, we
+    /// only allow watchdog to pause the bridge, but does not allow it to unpause the bridge.
+    function _authorizePause(address addr, bool toPause) internal view virtual override {
+        // Owenr and chain_pauser can pause/unpause the bridge.
+        if (addr == owner() || addr == resolve("chain_pauser", true)) return;
+
+        // bridge_watchdog can pause the bridge, but cannot unpause it.
+        if (toPause && addr == resolve("bridge_watchdog", true)) return;
+
+        revert RESOLVER_DENIED();
+    }
 
     /// @notice Invokes a call message on the Bridge.
     /// @param _message The call message to be invoked.

diff --git a/packages/protocol/contracts/common/AddressManager.sol b/packages/protocol/contracts/common/AddressManager.sol
@@ -55,7 +55,7 @@ contract AddressManager is EssentialContract, IAddressManager {
         return __addresses[_chainId][_name];
     }
 
-    function _authorizePause(address) internal pure override {
+    function _authorizePause(address, bool) internal pure override {
         revert AM_UNSUPPORTED();
     }
 }
diff --git a/packages/protocol/contracts/common/EssentialContract.sol b/packages/protocol/contracts/common/EssentialContract.sol
@@ -72,7 +72,7 @@ abstract contract EssentialContract is UUPSUpgradeable, Ownable2StepUpgradeable,
         emit Paused(msg.sender);
         // We call the authorize function here to avoid:
         // Warning (5740): Unreachable code.
-        _authorizePause(msg.sender);
+        _authorizePause(msg.sender, true);
     }
 
     /// @notice Unpauses the contract.
@@ -81,7 +81,7 @@ abstract contract EssentialContract is UUPSUpgradeable, Ownable2StepUpgradeable,
         emit Unpaused(msg.sender);
         // We call the authorize function here to avoid:
         // Warning (5740): Unreachable code.
-        _authorizePause(msg.sender);
+        _authorizePause(msg.sender, false);
     }
 
     /// @notice Returns true if the contract is paused, and false otherwise.
@@ -114,7 +114,7 @@ abstract contract EssentialContract is UUPSUpgradeable, Ownable2StepUpgradeable,
 
     function _authorizeUpgrade(address) internal virtual override onlyOwner { }
 
-    function _authorizePause(address) internal virtual onlyOwner { }
+    function _authorizePause(address, bool) internal virtual onlyOwner { }
 
     // Stores the reentry lock
     function _storeReentryLock(uint8 _reentry) internal virtual {

diff --git a/packages/protocol/contracts/signal/SignalService.sol b/packages/protocol/contracts/signal/SignalService.sol
@@ -227,7 +227,7 @@ contract SignalService is EssentialContract, ISignalService {
         );
     }
 
-    function _authorizePause(address) internal pure override {
+    function _authorizePause(address, bool) internal pure override {
         revert SS_UNSUPPORTED();
     }