Use constructor in ProposalTypesConfigurator (#64)

* Use constructor

* Update tests

src/AgoraGovernor.sol

@@ -126,22 +126,18 @@ contract AgoraGovernor is
      * @param _manager Manager address.
      * @param _timelock The governance timelock.
      * @param _proposalTypesConfigurator Proposal types configurator contract.
-     * @param _proposalTypes Initial proposal types to set.
      */
     function initialize(
         IVotingToken _votingToken,
         SupplyType _supplyType,
         address _admin,
         address _manager,
         TimelockControllerUpgradeable _timelock,
-        IProposalTypesConfigurator _proposalTypesConfigurator,
-        IProposalTypesConfigurator.ProposalType[] calldata _proposalTypes
+        IProposalTypesConfigurator _proposalTypesConfigurator
     ) public initializer {
         PROPOSAL_TYPES_CONFIGURATOR = _proposalTypesConfigurator;
         SUPPLY_TYPE = _supplyType;
 
-        PROPOSAL_TYPES_CONFIGURATOR.initialize(address(this), _proposalTypes);
-
         __Governor_init("Agora");
         __GovernorCountingSimple_init();
         __GovernorVotes_init(_votingToken);

src/ProposalTypesConfigurator.sol

@@ -21,7 +21,7 @@ contract ProposalTypesConfigurator is IProposalTypesConfigurator {
                            IMMUTABLE STORAGE
     //////////////////////////////////////////////////////////////*/
 
-    IAgoraGovernor public governor;
+    IAgoraGovernor public immutable GOVERNOR;
 
     /// @notice Max value of `quorum` and `approvalThreshold` in `ProposalType`
     uint16 public constant PERCENT_DIVISOR = 10_000;
@@ -39,24 +39,23 @@ contract ProposalTypesConfigurator is IProposalTypesConfigurator {
     //////////////////////////////////////////////////////////////*/
 
     modifier onlyAdminOrTimelock() {
-        if (msg.sender != governor.admin() && msg.sender != governor.timelock()) revert NotAdminOrTimelock();
+        if (msg.sender != GOVERNOR.admin() && msg.sender != GOVERNOR.timelock()) revert NotAdminOrTimelock();
         _;
     }
 
     /*//////////////////////////////////////////////////////////////
-                               FUNCTIONS
+                              CONSTRUCTOR
     //////////////////////////////////////////////////////////////*/
 
     /**
      * @notice Initialize the contract with the governor and proposal types.
      * @param _governor Address of the governor contract.
      * @param _proposalTypesInit Array of ProposalType structs to initialize the contract with.
      */
-    function initialize(address _governor, ProposalType[] calldata _proposalTypesInit) external {
-        if (address(governor) != address(0)) revert AlreadyInit();
+    constructor(address _governor, ProposalType[] memory _proposalTypesInit) {
         if (_governor == address(0)) revert InvalidGovernor();
 
-        governor = IAgoraGovernor(_governor);
+        GOVERNOR = IAgoraGovernor(_governor);
 
         for (uint8 i = 0; i < _proposalTypesInit.length; i++) {
             _setProposalType(
@@ -70,6 +69,10 @@ contract ProposalTypesConfigurator is IProposalTypesConfigurator {
         }
     }
 
+    /*//////////////////////////////////////////////////////////////
+                               FUNCTIONS
+    //////////////////////////////////////////////////////////////*/
+
     /**
      * @notice Get the parameters for a proposal type.
      * @param proposalTypeId Id of the proposal type.
@@ -151,8 +154,8 @@ contract ProposalTypesConfigurator is IProposalTypesConfigurator {
         uint8 proposalTypeId,
         uint16 quorum,
         uint16 approvalThreshold,
-        string calldata name,
-        string calldata description,
+        string memory name,
+        string memory description,
         address module
     ) internal {
         if (quorum > PERCENT_DIVISOR) revert InvalidQuorum();

src/interfaces/IProposalTypesConfigurator.sol

@@ -13,7 +13,6 @@ interface IProposalTypesConfigurator {
     error InvalidScope();
     error NotAdminOrTimelock();
     error NotAdmin();
-    error AlreadyInit();
     error InvalidGovernor();
     error Invalid4ByteSelector();
     error InvalidParamNotEqual();
@@ -80,8 +79,6 @@ interface IProposalTypesConfigurator {
                                FUNCTIONS
     //////////////////////////////////////////////////////////////*/
 
-    function initialize(address _governor, ProposalType[] calldata _proposalTypes) external;
-
     function proposalTypes(uint8 proposalTypeId) external view returns (ProposalType memory);
     function assignedScopes(uint8 proposalTypeId, bytes24 scopeKey) external view returns (Scope[] memory);
     function scopeExists(bytes24 key) external view returns (bool);

test/AgoraGovernor.t.sol

@@ -135,15 +135,18 @@ contract AgoraGovernorTest is Test {
             )
         );
 
-        // Deploy Proposal Types Configurator
-        proposalTypesConfigurator = new ProposalTypesConfigurator();
-
         // Deploy timelock
         timelock = Timelock(payable(new TransparentUpgradeableProxy(address(new Timelock()), proxyAdmin, "")));
 
         // Deploy governor impl
         implementation = address(new AgoraGovernorMock());
 
+        // Deploy Proposal Types Configurator
+        proposalTypesConfigurator = new ProposalTypesConfigurator(
+            vm.computeCreateAddress(deployer, vm.getNonce(deployer) + 1),
+            new IProposalTypesConfigurator.ProposalType[](0)
+        );
+
         // Deploy governor proxy
         governorProxy = address(
             new TransparentUpgradeableProxy(
@@ -157,8 +160,7 @@ contract AgoraGovernorTest is Test {
                         admin,
                         manager,
                         timelock,
-                        IProposalTypesConfigurator(proposalTypesConfigurator),
-                        new IProposalTypesConfigurator.ProposalType[](0)
+                        IProposalTypesConfigurator(proposalTypesConfigurator)
                     )
                 )
             )
@@ -278,7 +280,6 @@ contract Initialize is AgoraGovernorTest {
         public
         virtual
     {
-        ProposalTypesConfigurator _proposalTypesConfigurator = new ProposalTypesConfigurator();
         IProposalTypesConfigurator.ProposalType[] memory _proposalTypes =
             new IProposalTypesConfigurator.ProposalType[](4);
         _proposalTypes[0] =
@@ -289,6 +290,9 @@ contract Initialize is AgoraGovernorTest {
             IProposalTypesConfigurator.ProposalType(7_500, 3_100, "Whatever", "Lorem Ipsum", address(0), true);
         _proposalTypes[3] =
             IProposalTypesConfigurator.ProposalType(0, 0, "Optimistic", "Lorem Ipsum", address(optimisticModule), true);
+        ProposalTypesConfigurator _proposalTypesConfigurator = new ProposalTypesConfigurator(
+            vm.computeCreateAddress(address(this), vm.getNonce(address(this)) + 1), _proposalTypes
+        );
         AgoraGovernor _governor = AgoraGovernor(
             payable(
                 new TransparentUpgradeableProxy(
@@ -302,8 +306,7 @@ contract Initialize is AgoraGovernorTest {
                             _admin,
                             _manager,
                             TimelockControllerUpgradeable(payable(_timelock)),
-                            IProposalTypesConfigurator(_proposalTypesConfigurator),
-                            _proposalTypes
+                            IProposalTypesConfigurator(_proposalTypesConfigurator)
                         )
                     )
                 )

test/ProposalTypesConfigurator.t.sol

@@ -40,8 +40,8 @@ contract ProposalTypesConfiguratorTest is Test {
         governor = new GovernorMock(admin, timelock);
 
         vm.startPrank(deployer);
-        proposalTypesConfigurator = new ProposalTypesConfigurator();
-        proposalTypesConfigurator.initialize(address(governor), new ProposalTypesConfigurator.ProposalType[](0));
+        proposalTypesConfigurator =
+            new ProposalTypesConfigurator(address(governor), new ProposalTypesConfigurator.ProposalType[](0));
         vm.stopPrank();
 
         vm.startPrank(admin);
@@ -118,30 +118,23 @@ contract ProposalTypesConfiguratorTest is Test {
 contract Initialize is ProposalTypesConfiguratorTest {
     function test_SetsGovernor(address _actor, address _governor) public {
         vm.assume(_governor != address(0));
-        ProposalTypesConfigurator proposalTypesConfigurator = new ProposalTypesConfigurator();
-        vm.prank(_actor);
-        proposalTypesConfigurator.initialize(address(_governor), new ProposalTypesConfigurator.ProposalType[](0));
-        assertEq(_governor, address(proposalTypesConfigurator.governor()));
+        ProposalTypesConfigurator proposalTypesConfigurator =
+            new ProposalTypesConfigurator(address(_governor), new ProposalTypesConfigurator.ProposalType[](0));
+        assertEq(_governor, address(proposalTypesConfigurator.GOVERNOR()));
     }
 
     function test_SetsProposalTypes(address _actor, uint8 _proposalTypes) public {
-        ProposalTypesConfigurator proposalTypesConfigurator = new ProposalTypesConfigurator();
         ProposalTypesConfigurator.ProposalType[] memory proposalTypes =
             new ProposalTypesConfigurator.ProposalType[](_proposalTypes);
-        vm.prank(_actor);
-        proposalTypesConfigurator.initialize(address(governor), proposalTypes);
+        ProposalTypesConfigurator proposalTypesConfigurator =
+            new ProposalTypesConfigurator(address(governor), proposalTypes);
         for (uint8 i = 0; i < _proposalTypes; i++) {
             IProposalTypesConfigurator.ProposalType memory propType = proposalTypesConfigurator.proposalTypes(i);
             assertEq(propType.quorum, 0);
             assertEq(propType.approvalThreshold, 0);
             assertEq(propType.name, "");
         }
     }
-
-    function test_RevertIf_AlreadyInit() public {
-        vm.expectRevert(IProposalTypesConfigurator.AlreadyInit.selector);
-        proposalTypesConfigurator.initialize(address(governor), new ProposalTypesConfigurator.ProposalType[](0));
-    }
 }
 
 contract ProposalTypes is ProposalTypesConfiguratorTest {