Make immutable implementation of factories public (#2761)

Fixes #2759

solidity/contracts/hooks/aggregation/StaticAggregationHookFactory.sol

@@ -15,9 +15,9 @@ pragma solidity >=0.8.0;
 
 // ============ Internal Imports ============
 import {StaticAggregationHook} from "./StaticAggregationHook.sol";
-import {StaticNAddressSetFactory} from "../../libs/StaticNAddressSetFactory.sol";
+import {StaticAddressSetFactory} from "../../libs/StaticAddressSetFactory.sol";
 
-contract StaticAggregationHookFactory is StaticNAddressSetFactory {
+contract StaticAggregationHookFactory is StaticAddressSetFactory {
     function _deployImplementation()
         internal
         virtual

solidity/contracts/isms/aggregation/StaticAggregationIsmFactory.sol

@@ -2,9 +2,9 @@
 pragma solidity >=0.8.0;
 // ============ Internal Imports ============
 import {StaticAggregationIsm} from "./StaticAggregationIsm.sol";
-import {StaticMOfNAddressSetFactory} from "../../libs/StaticMOfNAddressSetFactory.sol";
+import {StaticThresholdAddressSetFactory} from "../../libs/StaticAddressSetFactory.sol";
 
-contract StaticAggregationIsmFactory is StaticMOfNAddressSetFactory {
+contract StaticAggregationIsmFactory is StaticThresholdAddressSetFactory {
     function _deployImplementation()
         internal
         virtual

solidity/contracts/isms/multisig/StaticMultisigIsm.sol

@@ -5,7 +5,7 @@ import {AbstractMultisigIsm} from "./AbstractMultisigIsm.sol";
 import {AbstractMerkleRootMultisigIsm} from "./AbstractMerkleRootMultisigIsm.sol";
 import {AbstractMessageIdMultisigIsm} from "./AbstractMessageIdMultisigIsm.sol";
 import {MetaProxy} from "../../libs/MetaProxy.sol";
-import {StaticMOfNAddressSetFactory} from "../../libs/StaticMOfNAddressSetFactory.sol";
+import {StaticThresholdAddressSetFactory} from "../../libs/StaticAddressSetFactory.sol";
 
 /**
  * @title AbstractMetaProxyMultisigIsm
@@ -55,13 +55,15 @@ contract StaticMessageIdMultisigIsm is
 
 // solhint-enable no-empty-blocks
 
-contract StaticMerkleRootMultisigIsmFactory is StaticMOfNAddressSetFactory {
+contract StaticMerkleRootMultisigIsmFactory is
+    StaticThresholdAddressSetFactory
+{
     function _deployImplementation() internal override returns (address) {
         return address(new StaticMerkleRootMultisigIsm());
     }
 }
 
-contract StaticMessageIdMultisigIsmFactory is StaticMOfNAddressSetFactory {
+contract StaticMessageIdMultisigIsmFactory is StaticThresholdAddressSetFactory {
     function _deployImplementation() internal override returns (address) {
         return address(new StaticMessageIdMultisigIsm());
     }

solidity/contracts/isms/routing/DomainRoutingIsmFactory.sol

@@ -33,9 +33,7 @@ abstract contract AbstractDomainRoutingIsmFactory {
         return _ism;
     }
 
-    // ============ Internal Functions ============
-
-    function implementation() internal view virtual returns (address);
+    function implementation() public view virtual returns (address);
 }
 
 /**
@@ -49,7 +47,7 @@ contract DomainRoutingIsmFactory is AbstractDomainRoutingIsmFactory {
         _implementation = address(new DomainRoutingIsm());
     }
 
-    function implementation() internal view override returns (address) {
+    function implementation() public view override returns (address) {
         return _implementation;
     }
 }
@@ -65,7 +63,7 @@ contract DefaultFallbackRoutingIsmFactory is AbstractDomainRoutingIsmFactory {
         _implementation = address(new DefaultFallbackRoutingIsm(mailbox));
     }
 
-    function implementation() internal view override returns (address) {
+    function implementation() public view override returns (address) {
         return _implementation;
     }
 }

solidity/contracts/libs/StaticMOfNAddressSetFactory.sol → solidity/contracts/libs/StaticAddressSetFactory.sol

@@ -7,28 +7,28 @@ import {Create2} from "@openzeppelin/contracts/utils/Create2.sol";
 // ============ Internal Imports ============
 import {MetaProxy} from "./MetaProxy.sol";
 
-abstract contract StaticMOfNAddressSetFactory {
+abstract contract StaticThresholdAddressSetFactory {
     // ============ Immutables ============
-    address private immutable _implementation;
+    address public immutable implementation;
 
     // ============ Constructor ============
 
     constructor() {
-        _implementation = _deployImplementation();
+        implementation = _deployImplementation();
     }
 
     function _deployImplementation() internal virtual returns (address);
 
     /**
-     * @notice Deploys a StaticMOfNAddressSet contract address for the given
+     * @notice Deploys a StaticThresholdAddressSet contract address for the given
      * values
      * @dev Consider sorting addresses to ensure contract reuse
      * @param _values An array of addresses
      * @param _threshold The threshold value to use
-     * @return set The contract address representing this StaticMOfNAddressSet
+     * @return set The contract address representing this StaticThresholdAddressSet
      */
     function deploy(address[] calldata _values, uint8 _threshold)
-        external
+        public
         returns (address)
     {
         (bytes32 _salt, bytes memory _bytecode) = _saltAndBytecode(
@@ -43,12 +43,12 @@ abstract contract StaticMOfNAddressSetFactory {
     }
 
     /**
-     * @notice Returns the StaticMOfNAddressSet contract address for the given
+     * @notice Returns the StaticThresholdAddressSet contract address for the given
      * values
      * @dev Consider sorting addresses to ensure contract reuse
      * @param _values An array of addresses
      * @param _threshold The threshold value to use
-     * @return set The contract address representing this StaticMOfNAddressSet
+     * @return set The contract address representing this StaticThresholdAddressSet
      */
     function getAddress(address[] calldata _values, uint8 _threshold)
         external
@@ -63,14 +63,14 @@ abstract contract StaticMOfNAddressSetFactory {
     }
 
     /**
-     * @notice Returns the StaticMOfNAddressSet contract address for the given
+     * @notice Returns the StaticThresholdAddressSet contract address for the given
      * values
      * @param _salt The salt used in Create2
      * @param _bytecode The metaproxy bytecode used in Create2
-     * @return set The contract address representing this StaticMOfNAddressSet
+     * @return set The contract address representing this StaticThresholdAddressSet
      */
     function _getAddress(bytes32 _salt, bytes memory _bytecode)
-        private
+        internal
         view
         returns (address)
     {
@@ -86,13 +86,45 @@ abstract contract StaticMOfNAddressSetFactory {
      * @return _bytecode The metaproxy bytecode used in Create2
      */
     function _saltAndBytecode(address[] calldata _values, uint8 _threshold)
-        private
+        internal
         view
         returns (bytes32, bytes memory)
     {
         bytes memory _metadata = abi.encode(_values, _threshold);
-        bytes memory _bytecode = MetaProxy.bytecode(_implementation, _metadata);
+        bytes memory _bytecode = MetaProxy.bytecode(implementation, _metadata);
         bytes32 _salt = keccak256(_metadata);
         return (_salt, _bytecode);
     }
 }
+
+abstract contract StaticAddressSetFactory is StaticThresholdAddressSetFactory {
+    /**
+     * @notice Deploys a StaticAddressSet contract address for the given
+     * values
+     * @dev Consider sorting addresses to ensure contract reuse
+     * @param _values An array of addresses
+     * @return set The contract address representing this StaticAddressSet
+     */
+    function deploy(address[] calldata _values) external returns (address) {
+        return super.deploy(_values, uint8(_values.length));
+    }
+
+    /**
+     * @notice Returns the StaticAddressSet contract address for the given
+     * values
+     * @dev Consider sorting addresses to ensure contract reuse
+     * @param _values An array of addresses
+     * @return set The contract address representing this StaticAddressSet
+     */
+    function getAddress(address[] calldata _values)
+        external
+        view
+        returns (address)
+    {
+        (bytes32 _salt, bytes memory _bytecode) = _saltAndBytecode(
+            _values,
+            uint8(_values.length)
+        );
+        return super._getAddress(_salt, _bytecode);
+    }
+}

solidity/test/isms/AggregationIsm.t.sol

@@ -6,7 +6,7 @@ import "forge-std/Test.sol";
 import {IAggregationIsm} from "../../contracts/interfaces/isms/IAggregationIsm.sol";
 import {StaticAggregationIsmFactory} from "../../contracts/isms/aggregation/StaticAggregationIsmFactory.sol";
 import {AggregationIsmMetadata} from "../../contracts/isms/libs/AggregationIsmMetadata.sol";
-import {TestIsm, MOfNTestUtils} from "./IsmTestUtils.sol";
+import {TestIsm, ThresholdTestUtils} from "./IsmTestUtils.sol";
 
 contract AggregationIsmTest is Test {
     StaticAggregationIsmFactory factory;
@@ -38,7 +38,7 @@ contract AggregationIsmTest is Test {
         returns (bytes memory)
     {
         (address[] memory choices, ) = ism.modulesAndThreshold("");
-        address[] memory chosen = MOfNTestUtils.choose(m, choices, seed);
+        address[] memory chosen = ThresholdTestUtils.choose(m, choices, seed);
         bytes memory offsets;
         uint32 start = 8 * uint32(choices.length);
         bytes memory metametadata;

solidity/test/isms/IsmTestUtils.sol

@@ -54,7 +54,7 @@ contract TestIsm is IInterchainSecurityModule {
     }
 }
 
-library MOfNTestUtils {
+library ThresholdTestUtils {
     function choose(
         uint8 m,
         uint256[] memory choices,

solidity/test/isms/MultisigIsm.t.sol

@@ -8,21 +8,21 @@ import {TestMailbox} from "../../contracts/test/TestMailbox.sol";
 import {StaticMerkleRootMultisigIsmFactory, StaticMessageIdMultisigIsmFactory} from "../../contracts/isms/multisig/StaticMultisigIsm.sol";
 import {MerkleRootMultisigIsmMetadata} from "../../contracts/isms/libs/MerkleRootMultisigIsmMetadata.sol";
 import {CheckpointLib} from "../../contracts/libs/CheckpointLib.sol";
-import {StaticMOfNAddressSetFactory} from "../../contracts/libs/StaticMOfNAddressSetFactory.sol";
+import {StaticThresholdAddressSetFactory} from "../../contracts/libs/StaticAddressSetFactory.sol";
 import {TypeCasts} from "../../contracts/libs/TypeCasts.sol";
 import {MerkleTreeHook} from "../../contracts/hooks/MerkleTreeHook.sol";
 import {TestMerkleTreeHook} from "../../contracts/test/TestMerkleTreeHook.sol";
 import {TestPostDispatchHook} from "../../contracts/test/TestPostDispatchHook.sol";
 import {Message} from "../../contracts/libs/Message.sol";
-import {MOfNTestUtils} from "./IsmTestUtils.sol";
+import {ThresholdTestUtils} from "./IsmTestUtils.sol";
 
 /// @notice since we removed merkle tree from the mailbox, we need to include the MerkleTreeHook in the test
 abstract contract AbstractMultisigIsmTest is Test {
     using Message for bytes;
     using TypeCasts for address;
 
     uint32 constant ORIGIN = 11;
-    StaticMOfNAddressSetFactory factory;
+    StaticThresholdAddressSetFactory factory;
     IMultisigIsm ism;
     TestMerkleTreeHook internal merkleTreeHook;
     TestPostDispatchHook internal noopHook;
@@ -42,7 +42,7 @@ abstract contract AbstractMultisigIsmTest is Test {
     ) internal returns (bytes memory) {
         uint32 domain = mailbox.localDomain();
         uint256[] memory keys = addValidators(m, n, seed);
-        uint256[] memory signers = MOfNTestUtils.choose(m, keys, seed);
+        uint256[] memory signers = ThresholdTestUtils.choose(m, keys, seed);
 
         (bytes32 root, uint32 index) = merkleTreeHook.latestCheckpoint();
         bytes32 messageId = message.id();

typescript/sdk/src/ism/HyperlaneIsmFactory.ts

@@ -8,7 +8,7 @@ import {
   IMultisigIsm__factory,
   IRoutingIsm__factory,
   StaticAggregationIsm__factory,
-  StaticMOfNAddressSetFactory,
+  StaticThresholdAddressSetFactory,
 } from '@hyperlane-xyz/core';
 import { Address, eqAddress, formatMessage, warn } from '@hyperlane-xyz/utils';
 
@@ -112,7 +112,7 @@ export class HyperlaneIsmFactory extends HyperlaneApp<IsmFactoryFactories> {
         ? this.getContracts(chain).merkleRootMultisigIsmFactory
         : this.getContracts(chain).messageIdMultisigIsmFactory;
 
-    const address = await this.deployMOfNFactory(
+    const address = await this.deployThresholdFactory(
       chain,
       multisigIsmFactory,
       config.validators,
@@ -179,7 +179,7 @@ export class HyperlaneIsmFactory extends HyperlaneApp<IsmFactoryFactories> {
     for (const module of config.modules) {
       addresses.push((await this.deploy(chain, module)).address);
     }
-    const address = await this.deployMOfNFactory(
+    const address = await this.deployThresholdFactory(
       chain,
       aggregationIsmFactory,
       addresses,
@@ -188,9 +188,9 @@ export class HyperlaneIsmFactory extends HyperlaneApp<IsmFactoryFactories> {
     return IAggregationIsm__factory.connect(address, signer);
   }
 
-  private async deployMOfNFactory(
+  private async deployThresholdFactory(
     chain: ChainName,
-    factory: StaticMOfNAddressSetFactory,
+    factory: StaticThresholdAddressSetFactory,
     values: Address[],
     threshold: number,
   ): Promise<Address> {