fix: revert explicitly on renounce

connext · Jun 5, 2023 · 85da8ed · 85da8ed
1 parent d06aa72
commit 85da8ed
Show file tree

Hide file tree

Showing 8 changed files with 28 additions and 11 deletions.
diff --git a/packages/deployments/contracts/contracts/messaging/MerkleTreeManager.sol b/packages/deployments/contracts/contracts/messaging/MerkleTreeManager.sol
@@ -12,6 +12,7 @@ import {MerkleLib} from "./libraries/MerkleLib.sol";
 contract MerkleTreeManager is ProposedOwnableUpgradeable {
   // ========== Custom Errors ===========
 
+  error MerkleTreeManager__renounceOwnership_prohibited();
   error MerkleTreeManager__setArborist_zeroAddress();
   error MerkleTreeManager__setArborist_alreadyArborist();
 
@@ -140,7 +141,9 @@ contract MerkleTreeManager is ProposedOwnableUpgradeable {
    * @dev Renounce ownership should be impossible as long as there is a possibility the
    * arborist may change.
    */
-  function renounceOwnership() public virtual override onlyOwner {}
+  function renounceOwnership() public virtual override onlyOwner {
+    revert MerkleTreeManager__renounceOwnership_prohibited();
+  }
 
   // ========= Public Functions =========
 

diff --git a/packages/deployments/contracts/contracts/messaging/RootManager.sol b/packages/deployments/contracts/contracts/messaging/RootManager.sol
@@ -134,6 +134,8 @@ contract RootManager is ProposedOwnable, IRootManager, WatcherClient, DomainInde
 
   error RootManager_constructor__DisputeBlocksLowerThanMin();
 
+  error RootManager__renounceOwnership_prohibited();
+
   // ============ Properties ============
 
   /**
@@ -394,7 +396,9 @@ contract RootManager is ProposedOwnable, IRootManager, WatcherClient, DomainInde
    * @dev Renounce ownership should be impossible as long as watchers can freely remove connectors
    * and only the owner can add them back
    */
-  function renounceOwnership() public virtual override(ProposedOwnable, WatcherClient) onlyOwner {}
+  function renounceOwnership() public virtual override(ProposedOwnable, WatcherClient) onlyOwner {
+    revert RootManager__renounceOwnership_prohibited();
+  }
 
   // ============ Public Functions ============
 

diff --git a/packages/deployments/contracts/contracts/messaging/WatcherClient.sol b/packages/deployments/contracts/contracts/messaging/WatcherClient.sol
@@ -63,7 +63,9 @@ contract WatcherClient is ProposedOwnable, Pausable {
    * is able to unpause the contracts. You can still propose `address(0)`,
    * but it will never be accepted.
    */
-  function renounceOwnership() public virtual override onlyOwner {}
+  function renounceOwnership() public virtual override onlyOwner {
+    require(false, "prohibited");
+  }
 
   // ============ Watcher fns ============
 

diff --git a/packages/deployments/contracts/contracts/messaging/WatcherManager.sol b/packages/deployments/contracts/contracts/messaging/WatcherManager.sol
@@ -48,5 +48,7 @@ contract WatcherManager is ProposedOwnable {
    * @dev Renounce ownership should be impossible as long as the watcher griefing
    * vector exists. You can still propose `address(0)`, but it will never be accepted.
    */
-  function renounceOwnership() public virtual override onlyOwner {}
+  function renounceOwnership() public virtual override onlyOwner {
+    require(false, "prohibited");
+  }
 }
diff --git a/packages/deployments/contracts/contracts/messaging/connectors/SpokeConnector.sol b/packages/deployments/contracts/contracts/messaging/connectors/SpokeConnector.sol
@@ -309,7 +309,9 @@ abstract contract SpokeConnector is Connector, ConnectorManager, WatcherClient,
    * WatcherClient, and as long as only the owner can remove pending roots in case of
    * fraud.
    */
-  function renounceOwnership() public virtual override(ProposedOwnable, WatcherClient) onlyOwner {}
+  function renounceOwnership() public virtual override(ProposedOwnable, WatcherClient) onlyOwner {
+    require(false, "prohibited");
+  }
 
   // ============ Public Functions ============
 

diff --git a/...ages/deployments/contracts/contracts/messaging/connectors/gnosis/GnosisSpokeConnector.sol b/...ages/deployments/contracts/contracts/messaging/connectors/gnosis/GnosisSpokeConnector.sol
@@ -42,7 +42,9 @@ contract GnosisSpokeConnector is SpokeConnector, GnosisBase {
   /**
    * @notice Should not be able to renounce ownership
    */
-  function renounceOwnership() public virtual override(SpokeConnector, ProposedOwnable) onlyOwner {}
+  function renounceOwnership() public virtual override(SpokeConnector, ProposedOwnable) onlyOwner {
+    require(false, "prohibited");
+  }
 
   // ============ Private fns ============
   /**

diff --git a/...loyments/contracts/contracts/messaging/connectors/multichain/MultichainSpokeConnector.sol b/...loyments/contracts/contracts/messaging/connectors/multichain/MultichainSpokeConnector.sol
@@ -42,16 +42,16 @@ contract MultichainSpokeConnector is SpokeConnector, BaseMultichain {
   /**
    * @notice Should not be able to renounce ownership
    */
-  function renounceOwnership() public virtual override(SpokeConnector, ProposedOwnable) onlyOwner {}
+  function renounceOwnership() public virtual override(SpokeConnector, ProposedOwnable) onlyOwner {
+    require(false, "prohibited");
+  }
 
   // ============ Public fns ============
   /**
    * @notice Overrides `Connector.processMessage` to revert as multichain should
    * use `anyExecute` pathway instead.
    */
-  function processMessage(
-    bytes memory /*_data*/
-  ) external override {
+  function processMessage(bytes memory /*_data*/) external override {
     revert Connector__processMessage_notUsed();
   }
 

diff --git a/.../deployments/contracts/contracts/messaging/connectors/optimism/OptimismSpokeConnector.sol b/.../deployments/contracts/contracts/messaging/connectors/optimism/OptimismSpokeConnector.sol
@@ -46,7 +46,9 @@ contract OptimismSpokeConnector is SpokeConnector, BaseOptimism {
   /**
    * @notice Should not be able to renounce ownership
    */
-  function renounceOwnership() public virtual override(SpokeConnector, ProposedOwnable) onlyOwner {}
+  function renounceOwnership() public virtual override(SpokeConnector, ProposedOwnable) onlyOwner {
+    require(false, "prohibited");
+  }
 
   /**
    * @dev Sends `outboundRoot` to root manager on l1