refactor to clone with immutable args

gunboatsss · Oct 7, 2024 · 3fdc95b · 3fdc95b
1 parent 0918f08
commit 3fdc95b
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 91 deletions.
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -1,4 +1,6 @@
 {
   "solidity.packageDefaultDependenciesContractsDirectory": "src",
-  "solidity.packageDefaultDependenciesDirectory": "lib"
+  "solidity.packageDefaultDependenciesDirectory": "lib",
+  "solidity.compileUsingRemoteVersion": "v0.8.27+commit.40a35a09",
+  "solidity.remappings": ["solady/=lib/solady/src/", "forge-std/=lib/forge-std/src/"]
 }
diff --git a/report.md b/report.md
@@ -12,12 +12,13 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
 - [Low Issues](#low-issues)
   - [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners)
   - [L-2: Solidity pragma should be specific, not wide](#l-2-solidity-pragma-should-be-specific-not-wide)
-  - [L-3: Missing checks for `address(0)` when assigning values to address state variables](#l-3-missing-checks-for-address0-when-assigning-values-to-address-state-variables)
+  - [L-3: Define and use `constant` variables instead of using literals](#l-3-define-and-use-constant-variables-instead-of-using-literals)
   - [L-4: Event is missing `indexed` fields](#l-4-event-is-missing-indexed-fields)
   - [L-5: PUSH0 is not supported by all chains](#l-5-push0-is-not-supported-by-all-chains)
   - [L-6: Unused Custom Error](#l-6-unused-custom-error)
-  - [L-7: Costly operations inside loops.](#l-7-costly-operations-inside-loops)
-  - [L-8: State variable changes but no event is emitted.](#l-8-state-variable-changes-but-no-event-is-emitted)
+  - [L-7: Uninitialized local variables.](#l-7-uninitialized-local-variables)
+  - [L-8: Costly operations inside loops.](#l-8-costly-operations-inside-loops)
+  - [L-9: State variable changes but no event is emitted.](#l-9-state-variable-changes-but-no-event-is-emitted)
 
 
 # Summary
@@ -27,29 +28,29 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
 | Key | Value |
 | --- | --- |
 | .sol Files | 7 |
-| Total nSLOC | 361 |
+| Total nSLOC | 378 |
 
 
 ## Files Details
 
 | Filepath | nSLOC |
 | --- | --- |
-| src/AutoVeBribe.sol | 146 |
-| src/AutoVeBribeFactory.sol | 28 |
+| src/AutoVeBribe.sol | 153 |
+| src/AutoVeBribeFactory.sol | 38 |
 | src/interfaces/AutomationCompatibleInterface.sol | 5 |
 | src/interfaces/IOpsProxy.sol | 11 |
 | src/interfaces/IReward.sol | 42 |
 | src/interfaces/IVoter.sol | 105 |
 | src/libraries/ProtocolTimeLibrary.sol | 24 |
-| **Total** | **361** |
+| **Total** | **378** |
 
 
 ## Issue Summary
 
 | Category | No. of Issues |
 | --- | --- |
 | High | 1 |
-| Low | 8 |
+| Low | 9 |
 
 
 # High Issues
@@ -61,10 +62,10 @@ Consider protecting the initializer functions with modifiers.
 <details><summary>1 Found Instances</summary>
 
 
-- Found in src/AutoVeBribe.sol [Line: 38](src/AutoVeBribe.sol#L38)
+- Found in src/AutoVeBribe.sol [Line: 29](src/AutoVeBribe.sol#L29)
 
 	```solidity
-	    function initialize(address _gauge, address _owner) external {
+	    function initialize(address _owner) external {
 	```
 
 </details>
@@ -80,19 +81,19 @@ Contracts have owners with privileged rights to perform admin tasks and need to
 <details><summary>3 Found Instances</summary>
 
 
-- Found in src/AutoVeBribe.sol [Line: 15](src/AutoVeBribe.sol#L15)
+- Found in src/AutoVeBribe.sol [Line: 16](src/AutoVeBribe.sol#L16)
 
 	```solidity
 	contract AutoVeBribe is Ownable, AutomationCompatibleInterface {
 	```
 
-- Found in src/AutoVeBribe.sol [Line: 170](src/AutoVeBribe.sol#L170)
+- Found in src/AutoVeBribe.sol [Line: 176](src/AutoVeBribe.sol#L176)
 
 	```solidity
 	    function setTokenAmountPerEpoch(address _token, uint256 _amount) external onlyOwner {
 	```
 
-- Found in src/AutoVeBribe.sol [Line: 176](src/AutoVeBribe.sol#L176)
+- Found in src/AutoVeBribe.sol [Line: 182](src/AutoVeBribe.sol#L182)
 
 	```solidity
 	    function recoverERC20(address _token) external onlyOwner {
@@ -106,20 +107,8 @@ Contracts have owners with privileged rights to perform admin tasks and need to
 
 Consider using a specific version of Solidity in your contracts instead of a wide version. For example, instead of `pragma solidity ^0.8.0;`, use `pragma solidity 0.8.0;`
 
-<details><summary>7 Found Instances</summary>
-
-
-- Found in src/AutoVeBribe.sol [Line: 2](src/AutoVeBribe.sol#L2)
-
-	```solidity
-	pragma solidity >=0.8.27;
-	```
-
-- Found in src/AutoVeBribeFactory.sol [Line: 2](src/AutoVeBribeFactory.sol#L2)
+<details><summary>5 Found Instances</summary>
 
-	```solidity
-	pragma solidity >=0.8.27;
-	```
 
 - Found in src/interfaces/AutomationCompatibleInterface.sol [Line: 2](src/interfaces/AutomationCompatibleInterface.sol#L2)
 
@@ -155,29 +144,29 @@ Consider using a specific version of Solidity in your contracts instead of a wid
 
 
 
-## L-3: Missing checks for `address(0)` when assigning values to address state variables
+## L-3: Define and use `constant` variables instead of using literals
 
-Check for `address(0)` when assigning values to address state variables.
+If the same constant literal value is used multiple times, create a constant state variable and reference it throughout the contract.
 
-<details><summary>3 Found Instances</summary>
+<details><summary>4 Found Instances</summary>
 
 
-- Found in src/AutoVeBribe.sol [Line: 45](src/AutoVeBribe.sol#L45)
+- Found in src/AutoVeBribe.sol [Line: 36](src/AutoVeBribe.sol#L36)
 
 	```solidity
-	        bribeVotingReward = IReward(voter.gaugeToBribe(_gauge));
+	        bytes memory res = LibClone.argsOnClone(address(this), 0, 20);
 	```
 
-- Found in src/AutoVeBribe.sol [Line: 47](src/AutoVeBribe.sol#L47)
+- Found in src/AutoVeBribe.sol [Line: 43](src/AutoVeBribe.sol#L43)
 
 	```solidity
-	        gauge = _gauge;
+	        bytes memory res = LibClone.argsOnClone(address(this), 20, 40);
 	```
 
-- Found in src/AutoVeBribeFactory.sol [Line: 17](src/AutoVeBribeFactory.sol#L17)
+- Found in src/AutoVeBribe.sol [Line: 50](src/AutoVeBribe.sol#L50)
 
 	```solidity
-	        implementation = address(new AutoVeBribe(_voter));
+	        bytes memory res = LibClone.argsOnClone(address(this), 40, 60);
 	```
 
 </details>
@@ -235,20 +224,8 @@ Index event fields make the field more quickly accessible to off-chain tools tha
 
 Solc compiler version 0.8.20 switches the default target EVM version to Shanghai, which means that the generated bytecode will include PUSH0 opcodes. Be sure to select the appropriate EVM version in case you intend to deploy on a chain other than mainnet like L2 chains that may not support PUSH0, otherwise deployment of your contracts will fail.
 
-<details><summary>7 Found Instances</summary>
-
-
-- Found in src/AutoVeBribe.sol [Line: 2](src/AutoVeBribe.sol#L2)
-
-	```solidity
-	pragma solidity >=0.8.27;
-	```
-
-- Found in src/AutoVeBribeFactory.sol [Line: 2](src/AutoVeBribeFactory.sol#L2)
+<details><summary>5 Found Instances</summary>
 
-	```solidity
-	pragma solidity >=0.8.27;
-	```
 
 - Found in src/interfaces/AutomationCompatibleInterface.sol [Line: 2](src/interfaces/AutomationCompatibleInterface.sol#L2)
 
@@ -487,55 +464,72 @@ it is recommended that the definition be removed when custom error is unused
 
 
 
-## L-7: Costly operations inside loops.
+## L-7: Uninitialized local variables.
+
+Initialize all the variables. If a variable is meant to be initialized to zero, explicitly set it to zero to improve code readability.
+
+<details><summary>1 Found Instances</summary>
+
+
+- Found in src/AutoVeBribe.sol [Line: 168](src/AutoVeBribe.sol#L168)
+
+	```solidity
+	        uint256 i;
+	```
+
+</details>
+
+
+
+## L-8: Costly operations inside loops.
 
 Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local variable to hold the loop computation result.
 
 <details><summary>1 Found Instances</summary>
 
 
-- Found in src/AutoVeBribe.sol [Line: 163](src/AutoVeBribe.sol#L163)
+- Found in src/AutoVeBribe.sol [Line: 169](src/AutoVeBribe.sol#L169)
 
 	```solidity
-	        for (uint256 i = 0; i < _token.length; i++) {
+	        for (; i < _token.length; i++) {
 	```
 
 </details>
 
 
 
-## L-8: State variable changes but no event is emitted.
+## L-9: State variable changes but no event is emitted.
 
 State variable changes in this function but no event is emitted.
 
 <details><summary>5 Found Instances</summary>
 
 
-- Found in src/AutoVeBribe.sol [Line: 38](src/AutoVeBribe.sol#L38)
+- Found in src/AutoVeBribe.sol [Line: 29](src/AutoVeBribe.sol#L29)
 
 	```solidity
-	    function initialize(address _gauge, address _owner) external {
+	    function initialize(address _owner) external {
 	```
 
-- Found in src/AutoVeBribe.sol [Line: 123](src/AutoVeBribe.sol#L123)
+- Found in src/AutoVeBribe.sol [Line: 128](src/AutoVeBribe.sol#L128)
 
 	```solidity
 	    function performUpkeep(bytes calldata performData) external {
 	```
 
-- Found in src/AutoVeBribe.sol [Line: 129](src/AutoVeBribe.sol#L129)
+- Found in src/AutoVeBribe.sol [Line: 134](src/AutoVeBribe.sol#L134)
 
 	```solidity
 	    function distribute(address _token) public {
 	```
 
-- Found in src/AutoVeBribe.sol [Line: 162](src/AutoVeBribe.sol#L162)
+- Found in src/AutoVeBribe.sol [Line: 167](src/AutoVeBribe.sol#L167)
 
 	```solidity
 	    function distribute(address[] memory _token) public {
 	```
 
-- Found in src/AutoVeBribe.sol [Line: 170](src/AutoVeBribe.sol#L170)
+- Found in src/AutoVeBribe.sol [Line: 176](src/AutoVeBribe.sol#L176)
 
 	```solidity
 	    function setTokenAmountPerEpoch(address _token, uint256 _amount) external onlyOwner {

diff --git a/src/AutoVeBribe.sol b/src/AutoVeBribe.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity >=0.8.27;
+pragma solidity 0.8.27;
 
 import {IReward} from "./interfaces/IReward.sol";
 import {IVoter} from "./interfaces/IVoter.sol";
@@ -9,20 +9,17 @@ import {AutomationCompatibleInterface} from "./interfaces/AutomationCompatibleIn
 
 import {IOpsProxy} from "./interfaces/IOpsProxy.sol";
 
+import {LibClone} from "solady/utils/LibClone.sol";
 import {Ownable} from "solady/auth/Ownable.sol";
 import {SafeTransferLib} from "solady/utils/SafeTransferLib.sol";
 
 contract AutoVeBribe is Ownable, AutomationCompatibleInterface {
-    IVoter public immutable voter;
-
-    address public gauge;
-    IReward public bribeVotingReward;
+    bool public inited;
 
     mapping(address _token => uint256 amountPerEpoch) public amountToBribeByTokenPerEpoch;
     mapping(address _token => uint256 lastBribeTime) public nextBribeTimeByToken;
 
-    error GaugeAlreadySet();
-    error NotAGauge();
+    event SetNewAmountPerEpoch(address indexed token, uint256 indexed amount);
 
     error AlreadySentThisEpoch(address _token);
     error NotWhitelisted(address _token);
@@ -31,21 +28,31 @@ contract AutoVeBribe is Ownable, AutomationCompatibleInterface {
 
     error GaugeIsStillAlive();
 
-    constructor(address _voter) {
-        voter = IVoter(_voter);
+    function initialize(address _owner) external {
+        if(inited) revert AlreadyInitialized();
+        _initializeOwner(_owner);
+        inited = true;
     }
 
-    function initialize(address _gauge, address _owner) external {
-        if (gauge != address(0)) {
-            revert GaugeAlreadySet();
+    function voter() public view returns (IVoter _voter) {
+        bytes memory res = LibClone.argsOnClone(address(this), 0, 20);
+        assembly {
+            _voter := mload(add(res, 20))
         }
-        if (!voter.isGauge(_gauge)) {
-            revert NotAGauge();
+    }
+
+    function gauge() public view returns (address _gauge) {
+        bytes memory res = LibClone.argsOnClone(address(this), 20, 40);
+        assembly {
+            _gauge := mload(add(res, 20))
+        }
+    }
+
+    function bribeVotingReward() public view returns (IReward _bribeVotingReward) {
+        bytes memory res = LibClone.argsOnClone(address(this), 40, 60);
+        assembly {
+            _bribeVotingReward := mload(add(res, 20))
         }
-        bribeVotingReward = IReward(voter.gaugeToBribe(_gauge));
-        // console.log("initalizing bribe with ", address(bribeVotingReward));
-        gauge = _gauge;
-        _initializeOwner(_owner);
     }
 
     enum Err {
@@ -103,7 +110,7 @@ contract AutoVeBribe is Ownable, AutomationCompatibleInterface {
         uint256 tokenCount;
         for (uint256 i = 0; i < length; i++) {
             address token = tokens[i];
-            if (!voter.isWhitelistedToken(token)) {
+            if (!(voter().isWhitelistedToken(token))) {
                 continue;
             }
             if (SafeTransferLib.balanceOf(token, address(this)) > 0 && block.timestamp > nextBribeTimeByToken[token]) {
@@ -139,7 +146,7 @@ contract AutoVeBribe is Ownable, AutomationCompatibleInterface {
             revert AlreadySentThisEpoch(_token);
         }
 
-        if (!voter.isWhitelistedToken(_token)) {
+        if (!(voter().isWhitelistedToken(_token))) {
             revert NotWhitelisted(_token);
         }
 
@@ -155,8 +162,8 @@ contract AutoVeBribe is Ownable, AutomationCompatibleInterface {
         }
         nextBribeTimeByToken[_token] = ProtocolTimeLibrary.epochVoteEnd(block.timestamp);
 
-        SafeTransferLib.safeApprove(_token, address(bribeVotingReward), amountToSend);
-        bribeVotingReward.notifyRewardAmount(_token, amountToSend);
+        SafeTransferLib.safeApprove(_token, address(bribeVotingReward()), amountToSend);
+        bribeVotingReward().notifyRewardAmount(_token, amountToSend);
     }
 
     function distribute(address[] memory _token) public {
@@ -169,12 +176,13 @@ contract AutoVeBribe is Ownable, AutomationCompatibleInterface {
 
     function setTokenAmountPerEpoch(address _token, uint256 _amount) external onlyOwner {
         amountToBribeByTokenPerEpoch[_token] = _amount;
+        emit SetNewAmountPerEpoch(_token, _amount);
     }
 
     // ADMIN FUNCTION
 
     function recoverERC20(address _token) external onlyOwner {
-        if (voter.isWhitelistedToken(_token) && voter.isAlive(gauge)) {
+        if (voter().isWhitelistedToken(_token) && voter().isAlive(gauge())) {
             revert GaugeIsStillAlive();
         }
         SafeTransferLib.safeTransferAll(_token, msg.sender);