fix: `testRecipient` CLI bug (#3138)

### Description

- we were recovering `TestRecipient` from sdk artifacts which means the
deployer won't have ownership. So, I added a optional `shouldRecover`
boolean which I turned off for TestRecipient deployer (this is only for
our artifacts, if they have their own testRecipient, it will still
recover that if provided)
- moved `TestRecipientDeployer` from cli to sdk and removed duplicated
code from infra

### Drive-by changes

None

### Related issues

- fixes https://github.com/hyperlane-xyz/issues/issues/895

### Backward compatibility

Yes

### Testing

Manual (between anvil1 and optimismgoerli)

.changeset/dirty-ears-push.md

@@ -0,0 +1,7 @@
+---
+'@hyperlane-xyz/infra': patch
+'@hyperlane-xyz/cli': patch
+'@hyperlane-xyz/sdk': patch
+---
+
+Added `shouldRecover` flag to deployContractFromFactory so that the `TestRecipientDeployer` can deploy new contracts if it's not the owner of the prior deployments (We were recovering the SDK artifacts which meant the deployer won't be able to set the ISM as they needed)

typescript/cli/src/deploy/core.ts

@@ -21,6 +21,8 @@ import {
   MultiProvider,
   MultisigConfig,
   RoutingIsmConfig,
+  TestRecipientConfig,
+  TestRecipientDeployer,
   buildAgentConfig,
   buildAggregationIsmConfigs,
   defaultMultisigConfigs,
@@ -47,10 +49,6 @@ import {
   writeJson,
 } from '../utils/files.js';
 
-import {
-  TestRecipientConfig,
-  TestRecipientDeployer,
-} from './TestRecipientDeployer.js';
 import {
   isISMConfig,
   isZODISMConfig,

typescript/infra/scripts/deploy.ts

@@ -24,7 +24,6 @@ import { Contexts } from '../config/contexts';
 import { deployEnvToSdkEnv } from '../src/config/environment';
 import { deployWithArtifacts } from '../src/deployment/deploy';
 import { TestQuerySenderDeployer } from '../src/deployment/testcontracts/testquerysender';
-import { TestRecipientDeployer } from '../src/deployment/testcontracts/testrecipient';
 import { impersonateAccount, useLocalProvider } from '../src/utils/fork';
 
 import {
@@ -138,8 +137,7 @@ async function main() {
     );
     deployer = new LiquidityLayerDeployer(multiProvider);
   } else if (module === Modules.TEST_RECIPIENT) {
-    config = objMap(envConfig.core, (_chain) => true);
-    deployer = new TestRecipientDeployer(multiProvider);
+    throw new Error('Test recipient is not supported. Use CLI instead.');
   } else if (module === Modules.TEST_QUERY_SENDER) {
     // Get query router addresses
     const queryAddresses = getAddresses(

typescript/infra/src/deployment/testcontracts/testrecipient.ts

@@ -1,36 +0,0 @@
-import {
-  TestRecipient__factory,
-  TestTokenRecipient__factory,
-} from '@hyperlane-xyz/core';
-import {
-  ChainName,
-  HyperlaneDeployer,
-  MultiProvider,
-} from '@hyperlane-xyz/sdk';
-
-export const TEST_RECIPIENT_DEPLOYER_FACTORIES = {
-  TestRecipient: new TestRecipient__factory(),
-  TestTokenRecipient: new TestTokenRecipient__factory(),
-};
-
-export class TestRecipientDeployer extends HyperlaneDeployer<
-  never,
-  typeof TEST_RECIPIENT_DEPLOYER_FACTORIES
-> {
-  constructor(multiProvider: MultiProvider) {
-    super(multiProvider, TEST_RECIPIENT_DEPLOYER_FACTORIES);
-  }
-
-  async deployContracts(chain: ChainName) {
-    const TestRecipient = await this.deployContract(chain, 'TestRecipient', []);
-    const TestTokenRecipient = await this.deployContract(
-      chain,
-      'TestTokenRecipient',
-      [],
-    );
-    return {
-      TestRecipient,
-      TestTokenRecipient,
-    };
-  }
-}

typescript/sdk/src/core/TestRecipientDeployer.ts

@@ -1,13 +1,12 @@
 import debug from 'debug';
 
 import { TestRecipient, TestRecipient__factory } from '@hyperlane-xyz/core';
-import {
-  ChainName,
-  HyperlaneDeployer,
-  MultiProvider,
-} from '@hyperlane-xyz/sdk';
 import { Address, eqAddress } from '@hyperlane-xyz/utils';
 
+import { HyperlaneDeployer } from '../deploy/HyperlaneDeployer';
+import { MultiProvider } from '../providers/MultiProvider';
+import { ChainName } from '../types';
+
 export type TestRecipientConfig = {
   interchainSecurityModule: Address;
 };
@@ -39,7 +38,28 @@ export class TestRecipientDeployer extends HyperlaneDeployer<
     chain: ChainName,
     config: TestRecipientConfig,
   ): Promise<TestRecipientContracts> {
-    const testRecipient = await this.deployContract(chain, 'testRecipient', []);
+    const predeployed = this.readCache(
+      chain,
+      this.factories['testRecipient'],
+      'testRecipient',
+    );
+    let usePreviousDeployment = false;
+    if (
+      predeployed &&
+      eqAddress(
+        await predeployed.owner(),
+        await this.multiProvider.getSignerAddress(chain),
+      )
+    ) {
+      usePreviousDeployment = true;
+    }
+    const testRecipient = await this.deployContract(
+      chain,
+      'testRecipient',
+      [],
+      undefined,
+      usePreviousDeployment,
+    );
     try {
       this.logger(`Checking ISM ${chain}`);
       const ism = await testRecipient.interchainSecurityModule();
@@ -51,7 +71,11 @@ export class TestRecipientDeployer extends HyperlaneDeployer<
         const tx = testRecipient.setInterchainSecurityModule(
           config.interchainSecurityModule,
         );
-        await this.multiProvider.handleTx(chain, tx);
+        await this.runIfOwner(
+          chain,
+          testRecipient,
+          async () => await this.multiProvider.handleTx(chain, tx),
+        );
       }
     } catch (error) {
       this.logger(`Failed to check/update ISM for ${chain}: ${error}`);

typescript/sdk/src/deploy/HyperlaneDeployer.ts

@@ -300,20 +300,23 @@ export abstract class HyperlaneDeployer<
     contractName: string,
     constructorArgs: Parameters<F['deploy']>,
     initializeArgs?: Parameters<Awaited<ReturnType<F['deploy']>>['initialize']>,
+    shouldRecover = true,
   ): Promise<ReturnType<F['deploy']>> {
-    const cachedContract = this.readCache(chain, factory, contractName);
-    if (cachedContract) {
-      if (this.recoverVerificationInputs) {
-        const recoveredInputs = await this.recoverVerificationArtifacts(
-          chain,
-          contractName,
-          cachedContract,
-          constructorArgs,
-          initializeArgs,
-        );
-        this.addVerificationArtifacts(chain, recoveredInputs);
+    if (shouldRecover) {
+      const cachedContract = this.readCache(chain, factory, contractName);
+      if (cachedContract) {
+        if (this.recoverVerificationInputs) {
+          const recoveredInputs = await this.recoverVerificationArtifacts(
+            chain,
+            contractName,
+            cachedContract,
+            constructorArgs,
+            initializeArgs,
+          );
+          this.addVerificationArtifacts(chain, recoveredInputs);
+        }
+        return cachedContract;
       }
-      return cachedContract;
     }
 
     this.logger(`Deploy ${contractName} on ${chain}`);
@@ -347,6 +350,7 @@ export abstract class HyperlaneDeployer<
     initializeArgs?: Parameters<
       Awaited<ReturnType<Factories[K]['deploy']>>['initialize']
     >,
+    shouldRecover = true,
   ): Promise<HyperlaneContracts<Factories>[K]> {
     const contract = await this.deployContractFromFactory(
       chain,
@@ -354,6 +358,7 @@ export abstract class HyperlaneDeployer<
       contractName.toString(),
       constructorArgs,
       initializeArgs,
+      shouldRecover,
     );
     this.writeCache(chain, contractName, contract.address);
     return contract;

typescript/sdk/src/index.ts

@@ -59,6 +59,10 @@ export { HyperlaneCoreDeployer } from './core/HyperlaneCoreDeployer';
 export { MultiProtocolCore } from './core/MultiProtocolCore';
 export { TestCoreApp } from './core/TestCoreApp';
 export { TestCoreDeployer } from './core/TestCoreDeployer';
+export {
+  TestRecipientConfig,
+  TestRecipientDeployer,
+} from './core/TestRecipientDeployer';
 export { EvmCoreAdapter } from './core/adapters/EvmCoreAdapter';
 export { SealevelCoreAdapter } from './core/adapters/SealevelCoreAdapter';
 export { ICoreAdapter } from './core/adapters/types';