fix: ensure subgraph service contracts are owned by governor

Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
graphprotocol · Feb 25, 2025 · 830f7e8 · 830f7e8
1 parent 7187543
commit 830f7e8
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 9 deletions.
diff --git a/packages/subgraph-service/contracts/DisputeManager.sol b/packages/subgraph-service/contracts/DisputeManager.sol
@@ -103,20 +103,22 @@ contract DisputeManager is
 
     /**
      * @notice Initialize this contract.
+     * @param owner The owner of the contract
      * @param arbitrator Arbitrator role
      * @param disputePeriod Dispute period in seconds
      * @param disputeDeposit Deposit required to create a Dispute
      * @param fishermanRewardCut_ Percent of slashed funds for fisherman (ppm)
      * @param maxSlashingCut_ Maximum percentage of indexer stake that can be slashed (ppm)
      */
     function initialize(
+        address owner,
         address arbitrator,
         uint64 disputePeriod,
         uint256 disputeDeposit,
         uint32 fishermanRewardCut_,
         uint32 maxSlashingCut_
     ) external initializer {
-        __Ownable_init(msg.sender);
+        __Ownable_init(owner);
         __AttestationManager_init();
 
         _setArbitrator(arbitrator);

diff --git a/packages/subgraph-service/ignition/configs/protocol.default.json5 b/packages/subgraph-service/ignition/configs/protocol.default.json5
@@ -8,7 +8,8 @@
     "controllerAddress": "",
     "disputeManagerProxyAddress": "",
     "curationAddress": "",
-    "curationImplementationAddress": ""
+    "curationImplementationAddress": "",
+    "subgraphServiceProxyAddress": ""
   },
   "DisputeManager": {
     "disputePeriod": 2419200,
@@ -27,7 +28,6 @@
     "curationCut": 100000,
 
    // Must be set for step 2 of the deployment
-    "subgraphServiceProxyAddress": "",
     "subgraphServiceProxyAdminAddress": "",
     "graphTallyCollectorAddress": ""
   }

diff --git a/packages/subgraph-service/ignition/modules/DisputeManager.ts b/packages/subgraph-service/ignition/modules/DisputeManager.ts
@@ -7,8 +7,10 @@ import ProxyAdminArtifact from '@openzeppelin/contracts/build/contracts/ProxyAdm
 import TransparentUpgradeableProxyArtifact from '@openzeppelin/contracts/build/contracts/TransparentUpgradeableProxy.json'
 
 export default buildModule('DisputeManager', (m) => {
+  const deployer = m.getAccount(0)
   const governor = m.getParameter('governor')
   const controllerAddress = m.getParameter('controllerAddress')
+  const subgraphServiceProxyAddress = m.getParameter('subgraphServiceProxyAddress')
   const disputeManagerProxyAddress = m.getParameter('disputeManagerProxyAddress')
   const disputeManagerProxyAdminAddress = m.getParameter('disputeManagerProxyAdminAddress')
   const arbitrator = m.getParameter('arbitrator')
@@ -34,6 +36,7 @@ export default buildModule('DisputeManager', (m) => {
       name: 'DisputeManager',
       artifact: DisputeManagerArtifact,
       initArgs: [
+        deployer,
         arbitrator,
         disputePeriod,
         disputeDeposit,
@@ -42,7 +45,10 @@ export default buildModule('DisputeManager', (m) => {
       ],
     })
 
-  m.call(DisputeManagerProxyAdmin, 'transferOwnership', [governor], { after: [DisputeManager] })
+  const callSetSubgraphService = m.call(DisputeManager, 'setSubgraphService', [subgraphServiceProxyAddress])
+
+  m.call(DisputeManager, 'transferOwnership', [governor], { after: [callSetSubgraphService] })
+  m.call(DisputeManagerProxyAdmin, 'transferOwnership', [governor], { after: [callSetSubgraphService] })
 
   return {
     DisputeManager,

diff --git a/packages/subgraph-service/ignition/modules/SubgraphService.ts b/packages/subgraph-service/ignition/modules/SubgraphService.ts
@@ -49,6 +49,8 @@ export default buildModule('SubgraphService', (m) => {
   const callSetPauseGuardian = m.call(SubgraphService, 'setPauseGuardian', [pauseGuardian, true])
   const callSetMaxPOIStaleness = m.call(SubgraphService, 'setMaxPOIStaleness', [maxPOIStaleness])
   const callSetCurationCut = m.call(SubgraphService, 'setCurationCut', [curationCut])
+
+  m.call(SubgraphService, 'transferOwnership', [governor], { after: [callSetPauseGuardian, callSetMaxPOIStaleness, callSetCurationCut] })
   m.call(SubgraphServiceProxyAdmin, 'transferOwnership', [governor], { after: [callSetPauseGuardian, callSetMaxPOIStaleness, callSetCurationCut] })
 
   return {

diff --git a/packages/subgraph-service/tasks/deploy.ts b/packages/subgraph-service/tasks/deploy.ts
@@ -52,7 +52,7 @@ task('deploy:protocol', 'Deploy a new version of the Graph Protocol Horizon cont
     const horizonDeployment = await hre.ignition.deploy(HorizonModule, {
       displayUi: true,
       parameters: IgnitionHelper.patchConfig(HorizonConfig, {
-        SubgraphService: {
+        $global: {
           subgraphServiceProxyAddress: proxiesDeployment.Transparent_Proxy_SubgraphService.target as string,
         },
       }),
@@ -68,12 +68,12 @@ task('deploy:protocol', 'Deploy a new version of the Graph Protocol Horizon cont
           disputeManagerProxyAddress: proxiesDeployment.Transparent_Proxy_DisputeManager.target as string,
           curationAddress: horizonDeployment.Graph_Proxy_L2Curation.target as string,
           curationImplementationAddress: horizonDeployment.Implementation_L2Curation.target as string,
+          subgraphServiceProxyAddress: proxiesDeployment.Transparent_Proxy_SubgraphService.target as string,
         },
         DisputeManager: {
           disputeManagerProxyAdminAddress: proxiesDeployment.Transparent_ProxyAdmin_DisputeManager.target as string,
         },
         SubgraphService: {
-          subgraphServiceProxyAddress: proxiesDeployment.Transparent_Proxy_SubgraphService.target as string,
           subgraphServiceProxyAdminAddress: proxiesDeployment.Transparent_ProxyAdmin_SubgraphService.target as string,
           graphTallyCollectorAddress: horizonDeployment.GraphTallyCollector.target as string,
         },
@@ -170,8 +170,10 @@ function _patchStepConfig<ChainId extends number, ContractName extends string, H
       const GraphTallyCollector = horizonAddressBook.getEntry('GraphTallyCollector')
 
       patchedConfig = IgnitionHelper.patchConfig(config, {
-        SubgraphService: {
+        $global: {
           subgraphServiceProxyAddress: SubgraphService.address,
+        },
+        SubgraphService: {
           subgraphServiceProxyAdminAddress: SubgraphService.proxyAdmin,
           graphTallyCollectorAddress: GraphTallyCollector.address,
           disputeManagerProxyAddress: DisputeManager.address,

diff --git a/packages/subgraph-service/test/SubgraphBaseTest.t.sol b/packages/subgraph-service/test/SubgraphBaseTest.t.sol
@@ -137,7 +137,7 @@ abstract contract SubgraphBaseTest is Utils, Constants {
             users.governor,
             abi.encodeCall(
                 DisputeManager.initialize,
-                (users.arbitrator, disputePeriod, disputeDeposit, fishermanRewardPercentage, maxSlashingPercentage)
+                (users.deployer, users.arbitrator, disputePeriod, disputeDeposit, fishermanRewardPercentage, maxSlashingPercentage)
             )
         );
         disputeManager = DisputeManager(disputeManagerProxy);

diff --git a/packages/subgraph-service/test/disputeManager/constructor/constructor.t.sol b/packages/subgraph-service/test/disputeManager/constructor/constructor.t.sol
@@ -41,7 +41,7 @@ contract DisputeManagerConstructorTest is DisputeManagerTest {
                 users.governor,
                 abi.encodeCall(
                     DisputeManager.initialize,
-                    (arbitrator, disputePeriod, disputeDeposit, fishermanRewardPercentage, maxSlashingPercentage)
+                    (users.deployer, arbitrator, disputePeriod, disputeDeposit, fishermanRewardPercentage, maxSlashingPercentage)
                 )
             );
     }