feat: support for minimal proxies (#18)

**Motivation:** The TokenVotingFactory is over the 24kb size limit. We need to convert the token voting modules into minimal proxies and also support deterministic deployments. **Modifications:** * All token voting modules are now minimal proxies * Logic contracts are deployed in the script and then passed in as parameters to the Factory. * `_getCastDisapprovalTypedDataHash` in `TokenholderCaster` had a typo. Changed `role` to `support`. * Role is now stored as a public variable (Not immutable due to minimal proxies) in ActionCreator. And is passed in as a parameter in the Factory deploy. * Fix in `*bySig` tests. * Refactored test suite to be more modular. **Result:** Closes #4 Closes #11
llamaxyz · Dec 9, 2023 · 51c1890 · 51c1890
1 parent 962e579
commit 51c1890
Show file tree

Hide file tree

Showing 18 changed files with 2,063 additions and 2,027 deletions.
diff --git a/justfile b/justfile
@@ -18,8 +18,8 @@ run-script script_name flags='' sig='' args='':
     -vvvv {{flags}}
   mv _test test
 
-dry-run-deploy: (run-script 'DeployLlamaFactory')
+dry-run-deploy: (run-script 'DeployLlamaTokenVotingFactory')
 
-deploy: (run-script 'DeployLlamaFactory' '--broadcast --verify --build-info --build-info-path build_info')
+deploy: (run-script 'DeployLlamaTokenVotingFactory' '--broadcast --verify --build-info --build-info-path build_info')
 
-verify: (run-script 'DeployLlamaFactory' '--verify --resume')
+verify: (run-script 'DeployLlamaTokenVotingFactory' '--verify --resume')
diff --git a/script/DeployLlamaTokenVotingFactory.s.sol b/script/DeployLlamaTokenVotingFactory.s.sol
@@ -4,16 +4,60 @@ pragma solidity 0.8.23;
 import {Script} from "forge-std/Script.sol";
 
 import {DeployUtils} from "script/DeployUtils.sol";
+import {ERC20TokenholderActionCreator} from "src/token-voting/ERC20TokenholderActionCreator.sol";
+import {ERC20TokenholderCaster} from "src/token-voting/ERC20TokenholderCaster.sol";
+import {ERC721TokenholderActionCreator} from "src/token-voting/ERC721TokenholderActionCreator.sol";
+import {ERC721TokenholderCaster} from "src/token-voting/ERC721TokenholderCaster.sol";
 import {LlamaTokenVotingFactory} from "src/token-voting/LlamaTokenVotingFactory.sol";
 
-contract DeployLlamaFactory is Script {
+contract DeployLlamaTokenVotingFactory is Script {
+  // Logic contracts.
+  ERC20TokenholderActionCreator erc20TokenholderActionCreatorLogic;
+  ERC20TokenholderCaster erc20TokenholderCasterLogic;
+  ERC721TokenholderActionCreator erc721TokenholderActionCreatorLogic;
+  ERC721TokenholderCaster erc721TokenholderCasterLogic;
+
+  // Factory contracts.
   LlamaTokenVotingFactory tokenVotingFactory;
 
   function run() public {
-    DeployUtils.print(string.concat("Deploying Llama token voting factory to chain:", vm.toString(block.chainid)));
+    DeployUtils.print(
+      string.concat("Deploying Llama token voting factory and logic contracts to chain:", vm.toString(block.chainid))
+    );
+
+    vm.broadcast();
+    erc20TokenholderActionCreatorLogic = new ERC20TokenholderActionCreator();
+    DeployUtils.print(
+      string.concat("  ERC20TokenholderActionCreatorLogic: ", vm.toString(address(erc20TokenholderActionCreatorLogic)))
+    );
+
+    vm.broadcast();
+    erc20TokenholderCasterLogic = new ERC20TokenholderCaster();
+    DeployUtils.print(
+      string.concat("  ERC20TokenholderCasterLogic: ", vm.toString(address(erc20TokenholderCasterLogic)))
+    );
+
+    vm.broadcast();
+    erc721TokenholderActionCreatorLogic = new ERC721TokenholderActionCreator();
+    DeployUtils.print(
+      string.concat(
+        "  ERC721TokenholderActionCreatorLogic: ", vm.toString(address(erc721TokenholderActionCreatorLogic))
+      )
+    );
+
+    vm.broadcast();
+    erc721TokenholderCasterLogic = new ERC721TokenholderCaster();
+    DeployUtils.print(
+      string.concat("  ERC721TokenholderCasterLogic: ", vm.toString(address(erc721TokenholderCasterLogic)))
+    );
 
     vm.broadcast();
-    tokenVotingFactory = new LlamaTokenVotingFactory();
+    tokenVotingFactory = new LlamaTokenVotingFactory(
+      erc20TokenholderActionCreatorLogic,
+      erc20TokenholderCasterLogic,
+      erc721TokenholderActionCreatorLogic,
+      erc721TokenholderCasterLogic
+    );
     DeployUtils.print(string.concat("  LlamaTokenVotingFactory: ", vm.toString(address(tokenVotingFactory))));
   }
 }
diff --git a/src/token-voting/ERC20TokenholderActionCreator.sol b/src/token-voting/ERC20TokenholderActionCreator.sol
@@ -10,26 +10,43 @@ import {ERC20Votes} from "@openzeppelin/token/ERC20/extensions/ERC20Votes.sol";
 /// @notice This contract lets holders of a specified `ERC20Votes` token create actions on a llama instance if their
 /// token balance is greater than or equal to the creation threshold.
 contract ERC20TokenholderActionCreator is TokenholderActionCreator {
-  ERC20Votes public immutable TOKEN;
+  ERC20Votes public token;
 
-  constructor(ERC20Votes token, ILlamaCore llamaCore, uint256 _creationThreshold)
-    TokenholderActionCreator(llamaCore, _creationThreshold)
+  /// @dev This contract is deployed as a minimal proxy from the factory's `deployTokenVotingModule` function. The
+  /// `_disableInitializers` locks the implementation (logic) contract, preventing any future initialization of it.
+  constructor() {
+    _disableInitializers();
+  }
+
+  /// @notice Initializes a new `ERC20TokenholderActionCreator` clone.
+  /// @dev This function is called by the `deployTokenVotingModule` function in the `LlamaTokenVotingFactory` contract.
+  /// The `initializer` modifier ensures that this function can be invoked at most once.
+  /// @param _token The ERC20 token to be used for voting.
+  /// @param _llamaCore The `LlamaCore` contract for this Llama instance.
+  /// @param _role The role used by this contract to cast approvals and disapprovals.
+  /// @param _creationThreshold The default number of tokens required to create an action. This must
+  /// be in the same decimals as the token. For example, if the token has 18 decimals and you want a
+  /// creation threshold of 1000 tokens, pass in 1000e18.
+  function initialize(ERC20Votes _token, ILlamaCore _llamaCore, uint8 _role, uint256 _creationThreshold)
+    external
+    initializer
   {
-    TOKEN = token;
-    uint256 totalSupply = TOKEN.totalSupply();
+    __initializeTokenholderActionCreatorMinimalProxy(_llamaCore, _role, _creationThreshold);
+    token = _token;
+    uint256 totalSupply = token.totalSupply();
     if (totalSupply == 0) revert InvalidTokenAddress();
     if (_creationThreshold > totalSupply) revert InvalidCreationThreshold();
   }
 
   function _getPastVotes(address account, uint256 timestamp) internal view virtual override returns (uint256) {
-    return TOKEN.getPastVotes(account, timestamp);
+    return token.getPastVotes(account, timestamp);
   }
 
   function _getPastTotalSupply(uint256 timestamp) internal view virtual override returns (uint256) {
-    return TOKEN.getPastTotalSupply(timestamp);
+    return token.getPastTotalSupply(timestamp);
   }
 
   function _getClockMode() internal view virtual override returns (string memory) {
-    return TOKEN.CLOCK_MODE();
+    return token.CLOCK_MODE();
   }
 }
diff --git a/src/token-voting/ERC20TokenholderCaster.sol b/src/token-voting/ERC20TokenholderCaster.sol
@@ -10,25 +10,44 @@ import {ERC20Votes} from "@openzeppelin/token/ERC20/extensions/ERC20Votes.sol";
 /// @notice This contract lets holders of a given governance ERC20Votes token cast approvals and disapprovals
 /// on created actions.
 contract ERC20TokenholderCaster is TokenholderCaster {
-  ERC20Votes public immutable TOKEN;
+  ERC20Votes public token;
 
-  constructor(ERC20Votes token, ILlamaCore llamaCore, uint8 role, uint256 minApprovalPct, uint256 minDisapprovalPct)
-    TokenholderCaster(llamaCore, role, minApprovalPct, minDisapprovalPct)
-  {
-    TOKEN = token;
-    uint256 totalSupply = TOKEN.totalSupply();
+  /// @dev This contract is deployed as a minimal proxy from the factory's `deployTokenVotingModule` function. The
+  /// `_disableInitializers` locks the implementation (logic) contract, preventing any future initialization of it.
+  constructor() {
+    _disableInitializers();
+  }
+
+  /// @notice Initializes a new `ERC20TokenholderCaster` clone.
+  /// @dev This function is called by the `deployTokenVotingModule` function in the `LlamaTokenVotingFactory` contract.
+  /// The `initializer` modifier ensures that this function can be invoked at most once.
+  /// @param _token The ERC20 token to be used for voting.
+  /// @param _llamaCore The `LlamaCore` contract for this Llama instance.
+  /// @param _role The role used by this contract to cast approvals and disapprovals.
+  /// @param _minApprovalPct The minimum % of approvals required to submit approvals to `LlamaCore`.
+  /// @param _minDisapprovalPct The minimum % of disapprovals required to submit disapprovals to `LlamaCore`.
+  function initialize(
+    ERC20Votes _token,
+    ILlamaCore _llamaCore,
+    uint8 _role,
+    uint256 _minApprovalPct,
+    uint256 _minDisapprovalPct
+  ) external initializer {
+    __initializeTokenholderCasterMinimalProxy(_llamaCore, _role, _minApprovalPct, _minDisapprovalPct);
+    token = _token;
+    uint256 totalSupply = token.totalSupply();
     if (totalSupply == 0) revert InvalidTokenAddress();
   }
 
   function _getPastVotes(address account, uint256 timestamp) internal view virtual override returns (uint256) {
-    return TOKEN.getPastVotes(account, timestamp);
+    return token.getPastVotes(account, timestamp);
   }
 
   function _getPastTotalSupply(uint256 timestamp) internal view virtual override returns (uint256) {
-    return TOKEN.getPastTotalSupply(timestamp);
+    return token.getPastTotalSupply(timestamp);
   }
 
   function _getClockMode() internal view virtual override returns (string memory) {
-    return TOKEN.CLOCK_MODE();
+    return token.CLOCK_MODE();
   }
 }
diff --git a/src/token-voting/ERC721TokenholderActionCreator.sol b/src/token-voting/ERC721TokenholderActionCreator.sol
@@ -11,27 +11,44 @@ import {IERC721} from "@openzeppelin/token/ERC721/IERC721.sol";
 /// @notice This contract lets holders of a given governance ERC721Votes token create actions on the llama instance if
 /// they hold enough tokens.
 contract ERC721TokenholderActionCreator is TokenholderActionCreator {
-  ERC721Votes public immutable TOKEN;
+  ERC721Votes public token;
 
-  constructor(ERC721Votes token, ILlamaCore llamaCore, uint256 _creationThreshold)
-    TokenholderActionCreator(llamaCore, _creationThreshold)
+  /// @dev This contract is deployed as a minimal proxy from the factory's `deployTokenVotingModule` function. The
+  /// `_disableInitializers` locks the implementation (logic) contract, preventing any future initialization of it.
+  constructor() {
+    _disableInitializers();
+  }
+
+  /// @notice Initializes a new `ERC721TokenholderActionCreator` clone.
+  /// @dev This function is called by the `deployTokenVotingModule` function in the `LlamaTokenVotingFactory` contract.
+  /// The `initializer` modifier ensures that this function can be invoked at most once.
+  /// @param _token The ERC721 token to be used for voting.
+  /// @param _llamaCore The `LlamaCore` contract for this Llama instance.
+  /// @param _role The role used by this contract to cast approvals and disapprovals.
+  /// @param _creationThreshold The default number of tokens required to create an action. This must
+  /// be in the same decimals as the token. For example, if the token has 18 decimals and you want a
+  /// creation threshold of 1000 tokens, pass in 1000e18.
+  function initialize(ERC721Votes _token, ILlamaCore _llamaCore, uint8 _role, uint256 _creationThreshold)
+    external
+    initializer
   {
-    TOKEN = token;
-    if (!TOKEN.supportsInterface(type(IERC721).interfaceId)) revert InvalidTokenAddress();
-    uint256 totalSupply = TOKEN.getPastTotalSupply(block.timestamp - 1);
+    __initializeTokenholderActionCreatorMinimalProxy(_llamaCore, _role, _creationThreshold);
+    token = _token;
+    if (!token.supportsInterface(type(IERC721).interfaceId)) revert InvalidTokenAddress();
+    uint256 totalSupply = token.getPastTotalSupply(block.timestamp - 1);
     if (totalSupply == 0) revert InvalidTokenAddress();
     if (_creationThreshold > totalSupply) revert InvalidCreationThreshold();
   }
 
   function _getPastVotes(address account, uint256 timestamp) internal view virtual override returns (uint256) {
-    return TOKEN.getPastVotes(account, timestamp);
+    return token.getPastVotes(account, timestamp);
   }
 
   function _getPastTotalSupply(uint256 timestamp) internal view virtual override returns (uint256) {
-    return TOKEN.getPastTotalSupply(timestamp);
+    return token.getPastTotalSupply(timestamp);
   }
 
   function _getClockMode() internal view virtual override returns (string memory) {
-    return TOKEN.CLOCK_MODE();
+    return token.CLOCK_MODE();
   }
 }
diff --git a/src/token-voting/ERC721TokenholderCaster.sol b/src/token-voting/ERC721TokenholderCaster.sol
@@ -11,24 +11,43 @@ import {IERC721} from "@openzeppelin/token/ERC721/IERC721.sol";
 /// @notice This contract lets holders of a given governance ERC721Votes token cast approvals and disapprovals
 /// on created actions.
 contract ERC721TokenholderCaster is TokenholderCaster {
-  ERC721Votes public immutable TOKEN;
+  ERC721Votes public token;
 
-  constructor(ERC721Votes token, ILlamaCore llamaCore, uint8 role, uint256 minApprovalPct, uint256 minDisapprovalPct)
-    TokenholderCaster(llamaCore, role, minApprovalPct, minDisapprovalPct)
-  {
-    TOKEN = token;
-    if (!TOKEN.supportsInterface(type(IERC721).interfaceId)) revert InvalidTokenAddress();
+  /// @dev This contract is deployed as a minimal proxy from the factory's `deployTokenVotingModule` function. The
+  /// `_disableInitializers` locks the implementation (logic) contract, preventing any future initialization of it.
+  constructor() {
+    _disableInitializers();
+  }
+
+  /// @notice Initializes a new `ERC721TokenholderCaster` clone.
+  /// @dev This function is called by the `deployTokenVotingModule` function in the `LlamaTokenVotingFactory` contract.
+  /// The `initializer` modifier ensures that this function can be invoked at most once.
+  /// @param _token The ERC721 token to be used for voting.
+  /// @param _llamaCore The `LlamaCore` contract for this Llama instance.
+  /// @param _role The role used by this contract to cast approvals and disapprovals.
+  /// @param _minApprovalPct The minimum % of approvals required to submit approvals to `LlamaCore`.
+  /// @param _minDisapprovalPct The minimum % of disapprovals required to submit disapprovals to `LlamaCore`.
+  function initialize(
+    ERC721Votes _token,
+    ILlamaCore _llamaCore,
+    uint8 _role,
+    uint256 _minApprovalPct,
+    uint256 _minDisapprovalPct
+  ) external initializer {
+    __initializeTokenholderCasterMinimalProxy(_llamaCore, _role, _minApprovalPct, _minDisapprovalPct);
+    token = _token;
+    if (!token.supportsInterface(type(IERC721).interfaceId)) revert InvalidTokenAddress();
   }
 
   function _getPastVotes(address account, uint256 timestamp) internal view virtual override returns (uint256) {
-    return TOKEN.getPastVotes(account, timestamp);
+    return token.getPastVotes(account, timestamp);
   }
 
   function _getPastTotalSupply(uint256 timestamp) internal view virtual override returns (uint256) {
-    return TOKEN.getPastTotalSupply(timestamp);
+    return token.getPastTotalSupply(timestamp);
   }
 
   function _getClockMode() internal view virtual override returns (string memory) {
-    return TOKEN.CLOCK_MODE();
+    return token.CLOCK_MODE();
   }
 }