Transfer ownership in Mailbox initializer (#1477)

solidity/contracts/Mailbox.sol

@@ -112,11 +112,15 @@ contract Mailbox is
         localDomain = _localDomain;
     }
 
-    // ============ Initializer ============
+    // ============ Initializers ============
 
-    function initialize(address _defaultIsm) external initializer {
+    function initialize(address _owner, address _defaultIsm)
+        external
+        initializer
+    {
         __PausableReentrancyGuard_init();
         __Ownable_init();
+        transferOwnership(_owner);
         _setDefaultIsm(_defaultIsm);
     }
 

solidity/test/isms/multisigIsm.test.ts

@@ -366,7 +366,7 @@ describe('MultisigIsm', async () => {
       const destinationMailbox = await mailboxFactory.deploy(
         DESTINATION_DOMAIN,
       );
-      await destinationMailbox.initialize(multisigIsm.address);
+      await destinationMailbox.initialize(signer.address, multisigIsm.address);
       await destinationMailbox.process(metadata, message);
     });
 

solidity/test/mailbox.test.ts

@@ -35,13 +35,24 @@ describe('Mailbox', async () => {
     module = await moduleFactory.deploy();
     const mailboxFactory = new TestMailbox__factory(signer);
     mailbox = await mailboxFactory.deploy(originDomain);
-    await mailbox.initialize(module.address);
+    await mailbox.initialize(signer.address, module.address);
   });
 
-  it('Cannot be initialized twice', async () => {
-    await expect(mailbox.initialize(module.address)).to.be.revertedWith(
-      'Initializable: contract is already initialized',
-    );
+  describe('#initialize', () => {
+    it('Sets the owner', async () => {
+      const mailboxFactory = new TestMailbox__factory(signer);
+      mailbox = await mailboxFactory.deploy(originDomain);
+      const expectedOwner = nonOwner.address;
+      await mailbox.initialize(expectedOwner, module.address);
+      const owner = await mailbox.owner();
+      expect(owner).equals(expectedOwner);
+    });
+
+    it('Cannot be initialized twice', async () => {
+      await expect(
+        mailbox.initialize(signer.address, module.address),
+      ).to.be.revertedWith('Initializable: contract is already initialized');
+    });
   });
 
   describe('#dispatch', () => {

solidity/test/router.test.ts

@@ -79,7 +79,7 @@ describe('Router', async () => {
       await router.initialize(mailbox.address);
       const ism = await new TestIsm__factory(signer).deploy();
       await ism.setAccept(true);
-      await mailbox.initialize(ism.address);
+      await mailbox.initialize(signer.address, ism.address);
     });
 
     it('accepts message from enrolled mailbox and router', async () => {

typescript/infra/config/environments/test/core.ts

@@ -3,20 +3,24 @@ import { ChainMap, CoreConfig } from '@hyperlane-xyz/sdk';
 import { TestChains } from './chains';
 
 export const core: ChainMap<TestChains, CoreConfig> = {
-  // Hardhat accounts 1-4
+  // Owner is hardhat account 0
+  // Validators are hardhat accounts 1-3
   test1: {
+    owner: '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266',
     multisigIsm: {
       validators: ['0x70997970c51812dc3a010c7d01b50e0d17dc79c8'],
       threshold: 1,
     },
   },
   test2: {
+    owner: '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266',
     multisigIsm: {
       validators: ['0x3c44cdddb6a900fa2b585dd299e03d12fa4293bc'],
       threshold: 1,
     },
   },
   test3: {
+    owner: '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266',
     multisigIsm: {
       validators: ['0x90f79bf6eb2c4f870365e785982e1f101e93b906'],
       threshold: 1,

typescript/infra/config/environments/testnet3/core.ts

@@ -7,6 +7,7 @@ export const core: ChainMap<TestnetChains, CoreConfig> = objMap(
   validators,
   (_, validatorSet) => {
     return {
+      owner: '0xfaD1C94469700833717Fa8a3017278BC1cA8031C',
       multisigIsm: {
         validators: validatorSet.validators.map((v) => v.address),
         threshold: validatorSet.threshold,

typescript/sdk/src/core/TestCoreDeployer.ts

@@ -16,8 +16,9 @@ import { coreFactories } from './contracts';
 
 const nonZeroAddress = ethers.constants.AddressZero.replace('00', '01');
 
-// dummy config as TestInbox and TestOutbox do not use deployed ValidatorManager
+// dummy config as TestInbox and TestOutbox do not use deployed ISM
 const testMultisigIsmConfig: CoreConfig = {
+  owner: nonZeroAddress,
   multisigIsm: {
     validators: [nonZeroAddress],
     threshold: 1,

typescript/sdk/src/deploy/HyperlaneDeployer.ts

@@ -173,19 +173,25 @@ export abstract class HyperlaneDeployer<
         salt,
       );
 
-      const deployTx = deployOpts.initCalldata
-        ? await create2Factory.deployAndInit(
-            bytecode,
-            salt,
-            deployOpts.initCalldata,
-            chainConnection.overrides,
-          )
-        : await create2Factory.deploy(
-            bytecode,
-            salt,
-            chainConnection.overrides,
-          );
-      await chainConnection.handleTx(deployTx);
+      if ((await chainConnection.provider.getCode(contractAddr)) === '0x') {
+        const deployTx = deployOpts.initCalldata
+          ? await create2Factory.deployAndInit(
+              bytecode,
+              salt,
+              deployOpts.initCalldata,
+              chainConnection.overrides,
+            )
+          : await create2Factory.deploy(
+              bytecode,
+              salt,
+              chainConnection.overrides,
+            );
+        await chainConnection.handleTx(deployTx);
+      } else {
+        this.logger(
+          `Found contract deployed at CREATE2 address, skipping contract deploy`,
+        );
+      }
 
       this.verificationInputs[chain].push({
         name: contractName,
@@ -295,6 +301,7 @@ export abstract class HyperlaneDeployer<
         proxy.address,
         implementation.address,
         initData,
+        chainConnection.overrides,
       );
     } else {
       const constructorArgs: Parameters<

typescript/sdk/src/deploy/core/HyperlaneCoreDeployer.ts

@@ -68,13 +68,14 @@ export class HyperlaneCoreDeployer<
     deployOpts?: DeployOptions,
   ): Promise<ProxiedContract<Mailbox, TransparentProxyAddresses>> {
     const domain = chainMetadata[chain].id;
+    const owner = this.configMap[chain].owner;
 
     const mailbox = await this.deployProxiedContract(
       chain,
       'mailbox',
       [domain],
       proxyAdmin,
-      [defaultIsmAddress],
+      [owner, defaultIsmAddress],
       deployOpts,
     );
     return mailbox;
@@ -204,8 +205,8 @@ export class HyperlaneCoreDeployer<
     owner: types.Address,
     chainConnection: ChainConnection,
   ): Promise<ethers.ContractReceipt[]> {
+    // Mailbox ownership is transferred upon initialization.
     const ownables: Ownable[] = [
-      coreContracts.mailbox.contract,
       coreContracts.multisigIsm,
       coreContracts.proxyAdmin,
     ];

typescript/sdk/src/deploy/core/types.ts

@@ -11,7 +11,7 @@ export type MultisigIsmConfig = {
 
 export type CoreConfig = {
   multisigIsm: MultisigIsmConfig;
-  owner?: types.Address;
+  owner: types.Address;
   remove?: boolean;
 };