From 8e4f2bbe184deec00e2e84a038a562e961b79a72 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Mon, 2 Oct 2023 13:24:51 -0400 Subject: [PATCH] Make immutable implementation of factories public (#2761) Fixes https://github.com/hyperlane-xyz/hyperlane-monorepo/issues/2759 --- .../StaticAggregationHookFactory.sol | 4 +- .../StaticAggregationIsmFactory.sol | 4 +- .../isms/multisig/StaticMultisigIsm.sol | 8 +- .../isms/routing/DomainRoutingIsmFactory.sol | 8 +- ...actory.sol => StaticAddressSetFactory.sol} | 58 ++++++++++--- .../libs/StaticNAddressSetFactory.sol | 86 ------------------- solidity/test/isms/AggregationIsm.t.sol | 4 +- solidity/test/isms/IsmTestUtils.sol | 2 +- solidity/test/isms/MultisigIsm.t.sol | 8 +- typescript/sdk/src/ism/HyperlaneIsmFactory.ts | 10 +-- .../src/ism/HyperlaneIsmFactoryDeployer.ts | 16 ++++ 11 files changed, 85 insertions(+), 123 deletions(-) rename solidity/contracts/libs/{StaticMOfNAddressSetFactory.sol => StaticAddressSetFactory.sol} (62%) delete mode 100644 solidity/contracts/libs/StaticNAddressSetFactory.sol diff --git a/solidity/contracts/hooks/aggregation/StaticAggregationHookFactory.sol b/solidity/contracts/hooks/aggregation/StaticAggregationHookFactory.sol index 22fafb5441..c7c2d5a2c1 100644 --- a/solidity/contracts/hooks/aggregation/StaticAggregationHookFactory.sol +++ b/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 diff --git a/solidity/contracts/isms/aggregation/StaticAggregationIsmFactory.sol b/solidity/contracts/isms/aggregation/StaticAggregationIsmFactory.sol index 3bcd22250c..da4209499f 100644 --- a/solidity/contracts/isms/aggregation/StaticAggregationIsmFactory.sol +++ b/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 diff --git a/solidity/contracts/isms/multisig/StaticMultisigIsm.sol b/solidity/contracts/isms/multisig/StaticMultisigIsm.sol index f3f08efece..c634d05578 100644 --- a/solidity/contracts/isms/multisig/StaticMultisigIsm.sol +++ b/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()); } diff --git a/solidity/contracts/isms/routing/DomainRoutingIsmFactory.sol b/solidity/contracts/isms/routing/DomainRoutingIsmFactory.sol index f1b6d6e1f0..d0681723e3 100644 --- a/solidity/contracts/isms/routing/DomainRoutingIsmFactory.sol +++ b/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; } } diff --git a/solidity/contracts/libs/StaticMOfNAddressSetFactory.sol b/solidity/contracts/libs/StaticAddressSetFactory.sol similarity index 62% rename from solidity/contracts/libs/StaticMOfNAddressSetFactory.sol rename to solidity/contracts/libs/StaticAddressSetFactory.sol index 59ef461375..27276b7441 100644 --- a/solidity/contracts/libs/StaticMOfNAddressSetFactory.sol +++ b/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); + } +} diff --git a/solidity/contracts/libs/StaticNAddressSetFactory.sol b/solidity/contracts/libs/StaticNAddressSetFactory.sol deleted file mode 100644 index 14cb94ac4a..0000000000 --- a/solidity/contracts/libs/StaticNAddressSetFactory.sol +++ /dev/null @@ -1,86 +0,0 @@ -// SPDX-License-Identifier: MIT OR Apache-2.0 -pragma solidity >=0.8.0; -// ============ External Imports ============ -import {Address} from "@openzeppelin/contracts/utils/Address.sol"; -import {Create2} from "@openzeppelin/contracts/utils/Create2.sol"; - -// ============ Internal Imports ============ -import {MetaProxy} from "./MetaProxy.sol"; - -abstract contract StaticNAddressSetFactory { - // ============ Immutables ============ - address private immutable _implementation; - - // ============ Constructor ============ - - constructor() { - _implementation = _deployImplementation(); - } - - function _deployImplementation() internal virtual returns (address); - - /** - * @notice Deploys a StaticNAddressSet 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 StaticNAddressSet - */ - function deploy(address[] calldata _values) external returns (address) { - (bytes32 _salt, bytes memory _bytecode) = _saltAndBytecode(_values); - address _set = _getAddress(_salt, _bytecode); - if (!Address.isContract(_set)) { - _set = Create2.deploy(0, _salt, _bytecode); - } - return _set; - } - - /** - * @notice Returns the StaticNAddressSet 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 StaticNAddressSet - */ - function getAddress(address[] calldata _values) - external - view - returns (address) - { - (bytes32 _salt, bytes memory _bytecode) = _saltAndBytecode(_values); - return _getAddress(_salt, _bytecode); - } - - /** - * @notice Returns the StaticNAddressSet 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 StaticNAddressSet - */ - function _getAddress(bytes32 _salt, bytes memory _bytecode) - private - view - returns (address) - { - bytes32 _bytecodeHash = keccak256(_bytecode); - return Create2.computeAddress(_salt, _bytecodeHash); - } - - /** - * @notice Returns the create2 salt and bytecode for the given values - * @param _values An array of addresses - * @return _salt The salt used in Create2 - * @return _bytecode The metaproxy bytecode used in Create2 - */ - function _saltAndBytecode(address[] calldata _values) - private - view - returns (bytes32, bytes memory) - { - bytes memory _metadata = abi.encode(_values); - bytes memory _bytecode = MetaProxy.bytecode(_implementation, _metadata); - bytes32 _salt = keccak256(_metadata); - return (_salt, _bytecode); - } -} diff --git a/solidity/test/isms/AggregationIsm.t.sol b/solidity/test/isms/AggregationIsm.t.sol index 1b8075e98b..b747a65e80 100644 --- a/solidity/test/isms/AggregationIsm.t.sol +++ b/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; diff --git a/solidity/test/isms/IsmTestUtils.sol b/solidity/test/isms/IsmTestUtils.sol index d9a3d2daf7..f7f7e6b856 100644 --- a/solidity/test/isms/IsmTestUtils.sol +++ b/solidity/test/isms/IsmTestUtils.sol @@ -54,7 +54,7 @@ contract TestIsm is IInterchainSecurityModule { } } -library MOfNTestUtils { +library ThresholdTestUtils { function choose( uint8 m, uint256[] memory choices, diff --git a/solidity/test/isms/MultisigIsm.t.sol b/solidity/test/isms/MultisigIsm.t.sol index ea567e9def..286cd31668 100644 --- a/solidity/test/isms/MultisigIsm.t.sol +++ b/solidity/test/isms/MultisigIsm.t.sol @@ -8,13 +8,13 @@ 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 { @@ -22,7 +22,7 @@ abstract contract AbstractMultisigIsmTest is Test { 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(); diff --git a/typescript/sdk/src/ism/HyperlaneIsmFactory.ts b/typescript/sdk/src/ism/HyperlaneIsmFactory.ts index 133a6d2124..b7bdc9d15d 100644 --- a/typescript/sdk/src/ism/HyperlaneIsmFactory.ts +++ b/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 { ? 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 { 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 { return IAggregationIsm__factory.connect(address, signer); } - private async deployMOfNFactory( + private async deployThresholdFactory( chain: ChainName, - factory: StaticMOfNAddressSetFactory, + factory: StaticThresholdAddressSetFactory, values: Address[], threshold: number, ): Promise
{ diff --git a/typescript/sdk/src/ism/HyperlaneIsmFactoryDeployer.ts b/typescript/sdk/src/ism/HyperlaneIsmFactoryDeployer.ts index 44ba80e63d..6afd8f2ade 100644 --- a/typescript/sdk/src/ism/HyperlaneIsmFactoryDeployer.ts +++ b/typescript/sdk/src/ism/HyperlaneIsmFactoryDeployer.ts @@ -39,21 +39,37 @@ export class HyperlaneIsmFactoryDeployer extends HyperlaneDeployer< 'merkleRootMultisigIsmFactory', [], ); + this.verificationInputs[chain].push({ + name: 'StaticMerkleRootMultisigIsm', + address: await merkleRootMultisigIsmFactory.implementation(), + }); const messageIdMultisigIsmFactory = await this.deployContract( chain, 'messageIdMultisigIsmFactory', [], ); + this.verificationInputs[chain].push({ + name: 'StaticMessageIdMultisigIsm', + address: await messageIdMultisigIsmFactory.implementation(), + }); const aggregationIsmFactory = await this.deployContract( chain, 'aggregationIsmFactory', [], ); + this.verificationInputs[chain].push({ + name: 'StaticAggregationIsm', + address: await aggregationIsmFactory.implementation(), + }); const routingIsmFactory = await this.deployContract( chain, 'routingIsmFactory', [], ); + this.verificationInputs[chain].push({ + name: 'DomaingRoutingIsm', + address: await routingIsmFactory.implementation(), + }); return { merkleRootMultisigIsmFactory, messageIdMultisigIsmFactory,