feat(sdk): Support proxyAdmin checks for admins not owned by AW (#4338)

### Description
- Support checking proxyAdmin owners for non AW proxy admins
- Stop assuming all proxyAdmins will be owned by AW
- This change was motivated by the need to have our checker tooling
check Renzo Wapr routes, where Renzo owns the proxy routers

### Drive-by changes
- Clean up the interface for `ProxyAdminViolation`

### Testing

Manual

.changeset/grumpy-taxis-shout.md

@@ -0,0 +1,5 @@
+---
+'@hyperlane-xyz/sdk': minor
+---
+
+Support proxyAdmin checks for non AW owned warp router contracts

typescript/infra/scripts/check-deploy.ts

@@ -15,7 +15,7 @@ import {
   hypERC20factories,
   proxiedFactories,
 } from '@hyperlane-xyz/sdk';
-import { objFilter } from '@hyperlane-xyz/utils';
+import { eqAddress, objFilter } from '@hyperlane-xyz/utils';
 
 import { Contexts } from '../config/contexts.js';
 import { DEPLOYER } from '../config/environments/mainnet3/owners.js';
@@ -161,8 +161,21 @@ async function check() {
       .reduce((obj, key) => {
         obj[key] = {
           ...warpAddresses[key],
-          proxyAdmin: chainAddresses[key].proxyAdmin,
         };
+
+        // if the owner in the config is an AW account, set the proxyAdmin to the AW singleton proxyAdmin
+        // this will ensure that the checker will check that any proxies are owned by the singleton proxyAdmin
+        const proxyAdmin = eqAddress(
+          config[key].owner,
+          envConfig.owners[key].owner,
+        )
+          ? chainAddresses[key]?.proxyAdmin
+          : undefined;
+
+        if (proxyAdmin) {
+          obj[key].proxyAdmin = proxyAdmin;
+        }
+
         return obj;
       }, {} as typeof warpAddresses);
 

typescript/infra/src/govern/HyperlaneAppGovernor.ts

@@ -1,6 +1,7 @@
 import { BigNumber } from 'ethers';
 import prompts from 'prompts';
 
+import { ProxyAdmin__factory } from '@hyperlane-xyz/core';
 import { Ownable__factory } from '@hyperlane-xyz/core';
 import {
   ChainMap,
@@ -435,9 +436,9 @@ export abstract class HyperlaneAppGovernor<
   }
 
   async handleProxyAdminViolation(violation: ProxyAdminViolation) {
-    const code = await this.checker.multiProvider
-      .getProvider(violation.chain)
-      .getCode(violation.expectedProxyAdmin.address);
+    const provider = this.checker.multiProvider.getProvider(violation.chain);
+    const code = await provider.getCode(violation.expected);
+    const proxyAdminInterface = ProxyAdmin__factory.createInterface();
 
     let call;
     if (code !== '0x') {
@@ -446,17 +447,17 @@ export abstract class HyperlaneAppGovernor<
         chain: violation.chain,
         call: {
           to: violation.actual,
-          data: violation.expectedProxyAdmin.interface.encodeFunctionData(
-            'changeProxyAdmin',
-            [violation.proxy.address, violation.expected],
-          ),
+          data: proxyAdminInterface.encodeFunctionData('changeProxyAdmin', [
+            violation.proxyAddress,
+            violation.expected,
+          ]),
           value: BigNumber.from(0),
-          description: `Change proxyAdmin of transparent proxy ${violation.proxy.address} from ${violation.actual} to ${violation.expected}`,
+          description: `Change proxyAdmin of transparent proxy ${violation.proxyAddress} from ${violation.actual} to ${violation.expected}`,
         },
       };
     } else {
       throw new Error(
-        `Admin for proxy ${violation.proxy.address} is not a ProxyAdmin contract`,
+        `Admin for proxy ${violation.proxyAddress} is not a ProxyAdmin contract`,
       );
     }
 

typescript/sdk/src/core/HyperlaneCoreChecker.ts

@@ -41,7 +41,11 @@ export class HyperlaneCoreChecker extends HyperlaneAppChecker<
       return;
     }
 
-    await this.checkProxiedContracts(chain);
+    await this.checkProxiedContracts(
+      chain,
+      config.owner,
+      config.ownerOverrides,
+    );
     await this.checkMailbox(chain);
     await this.checkBytecodes(chain);
     await this.checkValidatorAnnounce(chain);

typescript/sdk/src/deploy/HyperlaneAppChecker.ts

@@ -81,41 +81,62 @@ export abstract class HyperlaneAppChecker<
 
   async checkProxiedContracts(
     chain: ChainName,
-    proxyAdminAddress?: Address,
+    owner: Address,
+    ownableOverrides?: Record<string, Address>,
   ): Promise<void> {
+    // expectedProxyAdminAddress may be undefined, this means that proxyAdmin is not set in the config/not known at deployment time
     const expectedProxyAdminAddress =
-      proxyAdminAddress ?? this.app.getContracts(chain).proxyAdmin.address;
-    if (!expectedProxyAdminAddress) {
-      throw new Error(
-        `Checking proxied contracts for ${chain} with no admin provided`,
-      );
-    }
+      this.app.getContracts(chain).proxyAdmin?.address;
     const provider = this.multiProvider.getProvider(chain);
-    const contracts = this.app.getContracts(chain);
-
-    const expectedProxyAdminContract = ProxyAdmin__factory.connect(
-      expectedProxyAdminAddress,
-      provider,
-    );
 
+    const contracts = this.app.getContracts(chain);
     await promiseObjAll(
       objMap(contracts, async (name, contract) => {
         if (await isProxy(provider, contract.address)) {
-          // Check the ProxiedContract's admin matches expectation
           const actualProxyAdminAddress = await proxyAdmin(
             provider,
             contract.address,
           );
-          if (!eqAddress(actualProxyAdminAddress, expectedProxyAdminAddress)) {
-            this.addViolation({
-              type: ViolationType.ProxyAdmin,
-              chain,
-              name,
-              expected: expectedProxyAdminAddress,
-              actual: actualProxyAdminAddress,
-              expectedProxyAdmin: expectedProxyAdminContract,
-              proxy: contract,
-            } as ProxyAdminViolation);
+
+          if (expectedProxyAdminAddress) {
+            // config defines an expected ProxyAdmin address, we therefore check if the actual ProxyAdmin address matches the expected one
+            if (
+              !eqAddress(actualProxyAdminAddress, expectedProxyAdminAddress)
+            ) {
+              this.addViolation({
+                type: ViolationType.ProxyAdmin,
+                chain,
+                name,
+                expected: expectedProxyAdminAddress,
+                actual: actualProxyAdminAddress,
+                proxyAddress: contract.address,
+              } as ProxyAdminViolation);
+            }
+          } else {
+            // config does not define an expected ProxyAdmin address, this means that checkOwnership will not be able to check the ownership of the ProxyAdmin contract
+            // as it is not explicitly defined in the config. We therefore check the ownership of the ProxyAdmin contract here.
+            const actualProxyAdminContract = ProxyAdmin__factory.connect(
+              actualProxyAdminAddress,
+              provider,
+            );
+            const actualProxyAdminOwner =
+              await actualProxyAdminContract.owner();
+            const expectedOwner = this.getOwner(
+              actualProxyAdminOwner,
+              'proxyAdmin',
+              ownableOverrides,
+            );
+            if (!eqAddress(actualProxyAdminOwner, expectedOwner)) {
+              const violation: OwnerViolation = {
+                chain,
+                name: 'proxyAdmin',
+                type: ViolationType.Owner,
+                actual: actualProxyAdminOwner,
+                expected: expectedOwner,
+                contract,
+              };
+              this.addViolation(violation);
+            }
           }
         }
       }),
@@ -181,6 +202,14 @@ export abstract class HyperlaneAppChecker<
     return bytecode.substring(0, bytecode.length - 90);
   }
 
+  private getOwner(
+    owner: Address,
+    contractName: string,
+    ownableOverrides?: Record<string, Address>,
+  ): Address {
+    return ownableOverrides?.[contractName] ?? owner;
+  }
+
   // This method checks whether the bytecode of a contract matches the expected bytecode. It forces the deployer to explicitly acknowledge a change in bytecode. The violations can be remediated by updating the expected bytecode hash.
   async checkBytecode(
     chain: ChainName,
@@ -229,7 +258,7 @@ export abstract class HyperlaneAppChecker<
   ): Promise<void> {
     const ownableContracts = await this.ownables(chain);
     for (const [name, contract] of Object.entries(ownableContracts)) {
-      const expectedOwner = ownableOverrides?.[name] ?? owner;
+      const expectedOwner = this.getOwner(owner, name, ownableOverrides);
       const actual = await contract.owner();
       if (!eqAddress(actual, expectedOwner)) {
         const violation: OwnerViolation = {

typescript/sdk/src/deploy/types.ts

@@ -4,9 +4,7 @@ import { z } from 'zod';
 import type {
   AccessControl,
   Ownable,
-  ProxyAdmin,
   TimelockController,
-  TransparentUpgradeableProxy,
 } from '@hyperlane-xyz/core';
 import { Address } from '@hyperlane-xyz/utils';
 
@@ -43,8 +41,7 @@ export interface OwnerViolation extends CheckerViolation {
 
 export interface ProxyAdminViolation extends CheckerViolation {
   type: ViolationType.ProxyAdmin;
-  expectedProxyAdmin: ProxyAdmin;
-  proxy: TransparentUpgradeableProxy;
+  proxyAddress: Address;
   name: string;
 }
 

typescript/sdk/src/gas/HyperlaneIgpChecker.ts

@@ -22,7 +22,7 @@ export class HyperlaneIgpChecker extends HyperlaneAppChecker<
 > {
   async checkChain(chain: ChainName): Promise<void> {
     await this.checkDomainOwnership(chain);
-    await this.checkProxiedContracts(chain);
+    await this.checkProxiedContracts(chain, this.configMap[chain].owner);
     await this.checkBytecodes(chain);
     await this.checkOverheadInterchainGasPaymaster(chain);
     await this.checkInterchainGasPaymaster(chain);

typescript/sdk/src/router/ProxiedRouterChecker.ts

@@ -1,3 +1,4 @@
+import { AddressesMap } from '../contracts/types.js';
 import { ChainName } from '../types.js';
 
 import { HyperlaneRouterChecker } from './HyperlaneRouterChecker.js';
@@ -9,16 +10,32 @@ export abstract class ProxiedRouterChecker<
   App extends RouterApp<Factories>,
   Config extends ProxiedRouterConfig,
 > extends HyperlaneRouterChecker<Factories, App, Config> {
-  async checkOwnership(chain: ChainName): Promise<void> {
+  getOwnableOverrides(chain: ChainName): AddressesMap | undefined {
     const config = this.configMap[chain];
-    let ownableOverrides = config.ownerOverrides;
-    if (config.timelock) {
+    let ownableOverrides = config?.ownerOverrides;
+    if (config?.timelock) {
       ownableOverrides = {
+        ...ownableOverrides,
         proxyAdmin: this.app.getAddresses(chain).timelockController,
       };
     }
+    return ownableOverrides;
+  }
+
+  async checkOwnership(chain: ChainName): Promise<void> {
+    return super.checkOwnership(
+      chain,
+      this.configMap[chain].owner,
+      this.getOwnableOverrides(chain),
+    );
+  }
 
-    return super.checkOwnership(chain, config.owner, ownableOverrides);
+  async checkProxiedContracts(chain: ChainName): Promise<void> {
+    return super.checkProxiedContracts(
+      chain,
+      this.configMap[chain].owner,
+      this.getOwnableOverrides(chain),
+    );
   }
 
   async checkChain(chain: ChainName): Promise<void> {