From cca3416549e93c909f7d54d378188cc2885f2e78 Mon Sep 17 00:00:00 2001
From: xeno097 <xeno097.cp@gmail.com>
Date: Fri, 25 Oct 2024 16:02:22 -0400
Subject: [PATCH] refactor(sdk): implemented the proxyAdminOwnershipUpdateTxs
 to dedup admin proxy ownership code

---
 typescript/sdk/src/core/EvmCoreModule.ts      |  6 +++
 typescript/sdk/src/deploy/proxy.ts            | 40 ++++++++++++++++++-
 .../sdk/src/token/EvmERC20WarpModule.ts       | 39 +++---------------
 3 files changed, 51 insertions(+), 34 deletions(-)

diff --git a/typescript/sdk/src/core/EvmCoreModule.ts b/typescript/sdk/src/core/EvmCoreModule.ts
index 33788a91c..d86548d83 100644
--- a/typescript/sdk/src/core/EvmCoreModule.ts
+++ b/typescript/sdk/src/core/EvmCoreModule.ts
@@ -27,6 +27,7 @@ import {
   ProxyFactoryFactories,
   proxyFactoryFactories,
 } from '../deploy/contracts.js';
+import { proxyAdminOwnershipUpdateTxs } from '../deploy/proxy.js';
 import { ContractVerifier } from '../deploy/verify/ContractVerifier.js';
 import { HookFactories } from '../hook/contracts.js';
 import { EvmIsmModule } from '../ism/EvmIsmModule.js';
@@ -95,6 +96,11 @@ export class EvmCoreModule extends HyperlaneModule<
     transactions.push(
       ...(await this.createDefaultIsmUpdateTxs(actualConfig, expectedConfig)),
       ...this.createMailboxOwnerUpdateTxs(actualConfig, expectedConfig),
+      ...proxyAdminOwnershipUpdateTxs(
+        actualConfig,
+        expectedConfig,
+        this.domainId,
+      ),
     );
 
     return transactions;
diff --git a/typescript/sdk/src/deploy/proxy.ts b/typescript/sdk/src/deploy/proxy.ts
index 041d27cac..29ec36d68 100644
--- a/typescript/sdk/src/deploy/proxy.ts
+++ b/typescript/sdk/src/deploy/proxy.ts
@@ -1,6 +1,11 @@
 import { ethers } from 'ethers';
 
-import { Address, eqAddress } from '@hyperlane-xyz/utils';
+import { Address, assert, eqAddress } from '@hyperlane-xyz/utils';
+
+import { AnnotatedEV5Transaction } from '../providers/ProviderType.js';
+
+import { EvmModuleDeployer } from './EvmModuleDeployer.js';
+import { DeployedOwnableConfig } from './types.js';
 
 export type UpgradeConfig = {
   timelock: {
@@ -67,3 +72,36 @@ export async function isProxy(
   const admin = await proxyAdmin(provider, proxy);
   return !eqAddress(admin, ethers.constants.AddressZero);
 }
+
+export function proxyAdminOwnershipUpdateTxs(
+  actualConfig: Readonly<{ proxyAdmin?: DeployedOwnableConfig }>,
+  expectedConfig: Readonly<{ proxyAdmin?: DeployedOwnableConfig }>,
+  chainId: number,
+): AnnotatedEV5Transaction[] {
+  const transactions: AnnotatedEV5Transaction[] = [];
+
+  // Return early because old config files did not have the
+  // proxyAdmin property
+  if (!expectedConfig.proxyAdmin?.address) {
+    return transactions;
+  }
+
+  const actualProxyAdmin = actualConfig.proxyAdmin!;
+  assert(
+    actualProxyAdmin.address === expectedConfig.proxyAdmin.address,
+    `ProxyAdmin contract addresses do not match. Expected ${expectedConfig.proxyAdmin.address}, got ${actualProxyAdmin.address}`,
+  );
+
+  transactions.push(
+    // Internally the createTransferOwnershipTx method already checks if the
+    // two owner values are the same and produces an empty tx batch if they are
+    ...EvmModuleDeployer.createTransferOwnershipTx({
+      actualOwner: actualProxyAdmin.owner,
+      expectedOwner: expectedConfig.proxyAdmin.owner,
+      deployedAddress: actualProxyAdmin.address!,
+      chainId: chainId,
+    }),
+  );
+
+  return transactions;
+}
diff --git a/typescript/sdk/src/token/EvmERC20WarpModule.ts b/typescript/sdk/src/token/EvmERC20WarpModule.ts
index 4c9f6b1f6..eafbf2536 100644
--- a/typescript/sdk/src/token/EvmERC20WarpModule.ts
+++ b/typescript/sdk/src/token/EvmERC20WarpModule.ts
@@ -20,6 +20,7 @@ import {
   HyperlaneModuleParams,
 } from '../core/AbstractHyperlaneModule.js';
 import { EvmModuleDeployer } from '../deploy/EvmModuleDeployer.js';
+import { proxyAdminOwnershipUpdateTxs } from '../deploy/proxy.js';
 import { EvmIsmModule } from '../ism/EvmIsmModule.js';
 import { DerivedIsmConfig } from '../ism/EvmIsmReader.js';
 import { MultiProvider } from '../providers/MultiProvider.js';
@@ -97,7 +98,11 @@ export class EvmERC20WarpModule extends HyperlaneModule<
       ...(await this.createIsmUpdateTxs(actualConfig, expectedConfig)),
       ...this.createRemoteRoutersUpdateTxs(actualConfig, expectedConfig),
       ...this.createOwnershipUpdateTxs(actualConfig, expectedConfig),
-      ...this.updateProxyAdminOwnershipTxs(actualConfig, expectedConfig),
+      ...proxyAdminOwnershipUpdateTxs(
+        actualConfig,
+        expectedConfig,
+        this.domainId,
+      ),
     );
 
     return transactions;
@@ -224,38 +229,6 @@ export class EvmERC20WarpModule extends HyperlaneModule<
     });
   }
 
-  updateProxyAdminOwnershipTxs(
-    actualConfig: Readonly<TokenRouterConfig>,
-    expectedConfig: Readonly<TokenRouterConfig>,
-  ): AnnotatedEV5Transaction[] {
-    const transactions: AnnotatedEV5Transaction[] = [];
-
-    // Return early because old warp config files did not have the
-    // proxyAdmin property
-    if (!expectedConfig.proxyAdmin) {
-      return transactions;
-    }
-
-    const actualProxyAdmin = actualConfig.proxyAdmin!;
-    assert(
-      actualProxyAdmin.address === expectedConfig.proxyAdmin.address,
-      `ProxyAdmin contract addresses do not match. Expected ${expectedConfig.proxyAdmin.address}, got ${actualProxyAdmin.address}`,
-    );
-
-    transactions.push(
-      // Internally the createTransferOwnershipTx method already checks if the
-      // two owner values are the same and produces an empty tx batch if they are
-      ...EvmModuleDeployer.createTransferOwnershipTx({
-        actualOwner: actualProxyAdmin.owner,
-        expectedOwner: expectedConfig.proxyAdmin.owner,
-        deployedAddress: actualProxyAdmin.address!,
-        chainId: this.domainId,
-      }),
-    );
-
-    return transactions;
-  }
-
   /**
    * Updates or deploys the ISM using the provided configuration.
    *