From 67a6d971edc2e0b0d9849327f2cbbd63e7d7544c Mon Sep 17 00:00:00 2001 From: Kunal Arora <55632507+aroralanuk@users.noreply.github.com> Date: Tue, 9 Jan 2024 15:27:18 -0500 Subject: [PATCH] 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 | 7 ++++ typescript/cli/src/deploy/core.ts | 6 +-- typescript/infra/scripts/deploy.ts | 4 +- .../deployment/testcontracts/testrecipient.ts | 36 ------------------ .../src/core}/TestRecipientDeployer.ts | 38 +++++++++++++++---- .../sdk/src/deploy/HyperlaneDeployer.ts | 29 ++++++++------ typescript/sdk/src/index.ts | 4 ++ 7 files changed, 62 insertions(+), 62 deletions(-) create mode 100644 .changeset/dirty-ears-push.md delete mode 100644 typescript/infra/src/deployment/testcontracts/testrecipient.ts rename typescript/{cli/src/deploy => sdk/src/core}/TestRecipientDeployer.ts (66%) diff --git a/.changeset/dirty-ears-push.md b/.changeset/dirty-ears-push.md new file mode 100644 index 000000000..d1e81569c --- /dev/null +++ b/.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) diff --git a/typescript/cli/src/deploy/core.ts b/typescript/cli/src/deploy/core.ts index 47d0b688c..06a853716 100644 --- a/typescript/cli/src/deploy/core.ts +++ b/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, diff --git a/typescript/infra/scripts/deploy.ts b/typescript/infra/scripts/deploy.ts index cc39c893c..d9d58194f 100644 --- a/typescript/infra/scripts/deploy.ts +++ b/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( diff --git a/typescript/infra/src/deployment/testcontracts/testrecipient.ts b/typescript/infra/src/deployment/testcontracts/testrecipient.ts deleted file mode 100644 index fbb760c58..000000000 --- a/typescript/infra/src/deployment/testcontracts/testrecipient.ts +++ /dev/null @@ -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, - }; - } -} diff --git a/typescript/cli/src/deploy/TestRecipientDeployer.ts b/typescript/sdk/src/core/TestRecipientDeployer.ts similarity index 66% rename from typescript/cli/src/deploy/TestRecipientDeployer.ts rename to typescript/sdk/src/core/TestRecipientDeployer.ts index a62505bdd..6c5dc4246 100644 --- a/typescript/cli/src/deploy/TestRecipientDeployer.ts +++ b/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 { - 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}`); diff --git a/typescript/sdk/src/deploy/HyperlaneDeployer.ts b/typescript/sdk/src/deploy/HyperlaneDeployer.ts index 0daf8998e..0875bc4af 100644 --- a/typescript/sdk/src/deploy/HyperlaneDeployer.ts +++ b/typescript/sdk/src/deploy/HyperlaneDeployer.ts @@ -300,20 +300,23 @@ export abstract class HyperlaneDeployer< contractName: string, constructorArgs: Parameters, initializeArgs?: Parameters>['initialize']>, + shouldRecover = true, ): Promise> { - 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>['initialize'] >, + shouldRecover = true, ): Promise[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; diff --git a/typescript/sdk/src/index.ts b/typescript/sdk/src/index.ts index f6616b384..487a9685c 100644 --- a/typescript/sdk/src/index.ts +++ b/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';