fix: audit Trail of Bits - 23-12-2024

.github/workflows/forge-test.yml

@@ -37,10 +37,6 @@ jobs:
         run: forge fmt --check
         id: fmt
 
-      - name: Run natspec linting
-        run: bun lint:natspec
-        id: natspec
-
       - name: Run Forge build
         run: forge compile
         id: build

.gitmodules

@@ -13,21 +13,9 @@
 [submodule "lib/eas-contracts"]
 	path = lib/eas-contracts
 	url = https://github.com/ethereum-attestation-service/eas-contracts
-[submodule "lib/v2-core"]
-	path = lib/v2-core
-	url = https://github.com/sablier-labs/v2-core
 [submodule "lib/permit2"]
 	path = lib/permit2
 	url = https://github.com/Uniswap/permit2
-[submodule "lib/hedgey-vesting"]
-	path = lib/hedgey-vesting
-	url = https://github.com/hedgey-finance/Locked_VestingTokenPlans
-[submodule "lib/hats-protocol"]
-	path = lib/hats-protocol
-	url = https://github.com/Hats-Protocol/hats-protocol
 [submodule "lib/superfluid-protocol-monorepo"]
 	path = lib/superfluid-protocol-monorepo
 	url = https://github.com/superfluid-finance/protocol-monorepo
-[submodule "lib/eas-proxy"]
-	path = lib/eas-proxy
-	url = https://github.com/gitcoinco/eas-proxy

DEV.md

@@ -6,6 +6,12 @@
 git clone https://github.com/allo-protocol/allo-v2.1
 ```
 
+### Install submodules
+
+```bash
+git submodule update --init --recursive
+```
+
 ### Install bun
 
 ```bash

contracts/core/Allo.sol

@@ -322,6 +322,8 @@ contract Allo is IAllo, Initializable, Ownable, AccessControlUpgradeable, Reentr
 
         // Transfer the amount to the recipient (pool owner)
         _token.transferAmount(_recipient, _amount);
+
+        emit FundsRecovered(_token, _recipient);
     }
 
     // ====================================

contracts/core/Registry.sol

@@ -386,5 +386,7 @@ contract Registry is IRegistry, Initializable, AccessControlUpgradeable, Errors
 
         uint256 amount = _token.getBalance(address(this));
         _token.transferAmount(_recipient, amount);
+
+        emit FundsRecovered(_token, _recipient);
     }
 }

contracts/core/interfaces/IAllo.sol

@@ -103,6 +103,11 @@ interface IAllo {
     /// @param trustedForwarder Address of the new trusted forwarder
     event TrustedForwarderUpdated(address trustedForwarder);
 
+    /// @notice Emitted when funds are recovered
+    /// @param token The token recovered
+    /// @param recipient The recipient of the funds
+    event FundsRecovered(address token, address recipient);
+
     /// ====================================
     /// ==== External/Public Functions =====
     /// ====================================

contracts/core/interfaces/IRegistry.sol

@@ -88,6 +88,11 @@ interface IRegistry {
     /// @param pendingOwner The address of the pending owner
     event ProfilePendingOwnerUpdated(bytes32 indexed profileId, address pendingOwner);
 
+    /// @dev Emitted when funds are recovered
+    /// @param token The token recovered
+    /// @param recipient The recipient of the funds
+    event FundsRecovered(address token, address recipient);
+
     /// =========================
     /// ==== View Functions =====
     /// =========================

contracts/factories/ContractFactory.sol

@@ -97,14 +97,14 @@ contract ContractFactory {
         returns (address deployedContract)
     {
         // hash salt with the contract name and version
-        bytes32 salt = keccak256(abi.encodePacked(_contractName, _version));
+        bytes32 salt = keccak256(abi.encode(_contractName, _version));
 
         // ensure salt has not been used
         if (usedSalts[salt]) revert SALT_USED();
 
         usedSalts[salt] = true;
 
-        deployedContract = CREATE3.deploy(salt, creationCode, msg.value);
+        deployedContract = CREATE3.deployDeterministic(msg.value, creationCode, salt);
 
         emit Deployed(deployedContract, salt);
     }

contracts/strategies/examples/donation-voting/DonationVotingMerkleDistribution.sol

@@ -134,11 +134,10 @@ contract DonationVotingMerkleDistribution is DonationVotingOffchain {
     /// ====================================
 
     /// @notice Distributes funds (tokens) to recipients.
-    /// @param _recipientIds NOT USED
     /// @param _data Data to be decoded
     /// @custom:data (Distribution[] _distributions)
     /// @param _sender The address of the sender
-    function _distribute(address[] memory _recipientIds, bytes memory _data, address _sender)
+    function _distribute(address[] memory, bytes memory _data, address _sender)
         internal
         virtual
         override

contracts/strategies/examples/donation-voting/DonationVotingOffchain.sol

@@ -122,7 +122,6 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Allocation
     /// ===============================
 
     // @notice Initialize the strategy
-    /// @param _poolId ID of the pool
     /// @param _data The data to be decoded
     /// @custom:data (
     ///        RecipientInitializeData _recipientExtensionInitializeData,
@@ -132,7 +131,7 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Allocation
     ///        address[] _allowedTokens,
     ///        bool _isUsingAllocationMetadata
     ///    )
-    function _initializeStrategy(uint256 _poolId, bytes memory _data) internal override {
+    function _initializeStrategy(uint256, bytes memory _data) internal override {
         // Decode _data and initialize the specific strategy data
         (
             RecipientInitializeData memory _recipientExtensionInitializeData,
@@ -233,7 +232,7 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Allocation
         for (uint256 i; i < __recipients.length; i++) {
             if (!_isAcceptedRecipient(__recipients[i])) revert RecipientsExtension_RecipientNotAccepted();
 
-            if (!allowedTokens[_tokens[i]] && !allowedTokens[address(0)]) {
+            if (!allowedTokens[_tokens[i]] && !allowedTokens[ALL_TOKENS_ALLOWED]) {
                 revert DonationVotingOffchain_TokenNotAllowed();
             }
 
@@ -257,9 +256,8 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Allocation
 
     /// @notice Distributes funds (tokens) to recipients.
     /// @param _recipientIds The IDs of the recipients
-    /// @param _data NOT USED
     /// @param _sender The address of the sender
-    function _distribute(address[] memory _recipientIds, bytes memory _data, address _sender)
+    function _distribute(address[] memory _recipientIds, bytes memory, address _sender)
         internal
         virtual
         override
@@ -283,16 +281,12 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Allocation
     }
 
     /// @notice Hook called before withdrawing tokens from the pool.
-    /// @param _token The address of the token
-    /// @param _amount The amount to withdraw
-    /// @param _recipient The address to withdraw to
-    function _beforeWithdraw(address _token, uint256 _amount, address _recipient) internal virtual override {
+    function _beforeWithdraw(address, uint256, address) internal virtual override {
         if (block.timestamp <= allocationEndTime + withdrawalCooldown) revert INVALID();
     }
 
     /// @notice Hook called after increasing the pool amount.
-    /// @param _amount The amount to increase the pool by
-    function _beforeIncreasePoolAmount(uint256 _amount) internal virtual override {
+    function _beforeIncreasePoolAmount(uint256) internal virtual override {
         if (block.timestamp > allocationEndTime) revert AllocationExtension_ALLOCATION_HAS_ENDED();
     }
 
@@ -304,9 +298,8 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Allocation
     }
 
     /// @notice Returns always true as all addresses are valid allocators
-    /// @param _allocator NOT USED
     /// @return Returns always true
-    function _isValidAllocator(address _allocator) internal view override returns (bool) {
+    function _isValidAllocator(address) internal pure override returns (bool) {
         return true;
     }
 }

contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol

@@ -77,7 +77,6 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, AllocationE
     /// ===============================
 
     // @notice Initialize the strategy
-    /// @param _poolId ID of the pool
     /// @param _data The data to be decoded
     /// @custom:data (
     ///        RecipientInitializeData _recipientExtensionInitializeData,
@@ -87,7 +86,7 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, AllocationE
     ///        address _allocationToken,
     ///        bool _isUsingAllocationMetadata
     ///    )
-    function _initializeStrategy(uint256 _poolId, bytes memory _data) internal virtual override {
+    function _initializeStrategy(uint256, bytes memory _data) internal virtual override {
         (
             RecipientInitializeData memory _recipientExtensionInitializeData,
             uint64 _allocationStartTime,
@@ -183,16 +182,12 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, AllocationE
     }
 
     /// @notice Hook called before withdrawing tokens from the pool.
-    /// @param _token The address of the token
-    /// @param _amount The amount to withdraw
-    /// @param _recipient The address to withdraw to
-    function _beforeWithdraw(address _token, uint256 _amount, address _recipient) internal virtual override {
+    function _beforeWithdraw(address, uint256, address) internal virtual override {
         if (block.timestamp <= allocationEndTime + withdrawalCooldown) revert INVALID();
     }
 
     /// @notice Hook called before increasing the pool amount.
-    /// @param _amount The amount to increase the pool by
-    function _beforeIncreasePoolAmount(uint256 _amount) internal virtual override {
+    function _beforeIncreasePoolAmount(uint256) internal virtual override {
         if (block.timestamp > allocationEndTime) revert AllocationExtension_ALLOCATION_HAS_ENDED();
     }
 
@@ -204,9 +199,8 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, AllocationE
     }
 
     /// @notice Returns always true as all addresses are valid allocators
-    /// @param _allocator NOT USED
     /// @return Returns always true
-    function _isValidAllocator(address _allocator) internal view override returns (bool) {
+    function _isValidAllocator(address) internal pure override returns (bool) {
         return true;
     }
 }

contracts/strategies/examples/quadratic-voting/QVSimple.sol

@@ -65,9 +65,8 @@ contract QVSimple is BaseStrategy, RecipientsExtension, AllocatorsAllowlistExten
     /// ===============================
 
     /// @notice Initialize the strategy
-    /// @param _poolId The pool id
     /// @param _data The data to initialize the strategy (Must include RecipientInitializeData and QVSimpleInitializeData)
-    function _initializeStrategy(uint256 _poolId, bytes memory _data) internal virtual override {
+    function _initializeStrategy(uint256, bytes memory _data) internal virtual override {
         (
             IRecipientsExtension.RecipientInitializeData memory _recipientInitializeData,
             QVSimpleInitializeData memory _qvSimpleInitializeData
@@ -103,9 +102,8 @@ contract QVSimple is BaseStrategy, RecipientsExtension, AllocatorsAllowlistExten
     /// @notice Distribute the tokens to the recipients
     /// @dev The '_sender' must be a pool manager and the allocation must have ended
     /// @param _recipientIds The recipient ids
-    /// @param _data NOT USED
     /// @param _sender The sender of the transaction
-    function _distribute(address[] memory _recipientIds, bytes memory _data, address _sender)
+    function _distribute(address[] memory _recipientIds, bytes memory, address _sender)
         internal
         virtual
         override
@@ -194,8 +192,7 @@ contract QVSimple is BaseStrategy, RecipientsExtension, AllocatorsAllowlistExten
     }
 
     /// @notice Ensure no increase in pool amount is allowed after the distribution starts
-    /// @param _amount The amount to increase the pool by
-    function _beforeIncreasePoolAmount(uint256 _amount) internal virtual override {
+    function _beforeIncreasePoolAmount(uint256) internal virtual override {
         if (totalPayoutAmount != 0) {
             revert INVALID();
         }

contracts/strategies/examples/rfp/RFPSimple.sol

@@ -52,10 +52,9 @@ contract RFPSimple is BaseStrategy, MilestonesExtension, RecipientsExtension {
     /// ===============================
 
     // @notice Initialize the strategy
-    /// @param _poolId ID of the pool
     /// @param _data The data to be decoded
     /// @custom:data (RecipientInitializeData _recipientExtensionInitializeData, uint256 _maxBid)
-    function _initializeStrategy(uint256 _poolId, bytes memory _data) internal virtual override {
+    function _initializeStrategy(uint256, bytes memory _data) internal virtual override {
         (RecipientInitializeData memory _recipientExtensionInitializeData, uint256 _maxBid) =
             abi.decode(_data, (RecipientInitializeData, uint256));
         __RecipientsExtension_init(_recipientExtensionInitializeData);

contracts/strategies/examples/sqf-superfluid/RecipientSuperApp.sol

@@ -187,36 +187,27 @@ contract RecipientSuperApp is ISuperApp {
     /// ================================
 
     /// @dev This callback is called before the flow is created
-    /// @param superToken NOT USED
-    /// @param agreementClass NOT USED
-    /// @param agreementId NOT USED
-    /// @param agreementData NOT USED
-    /// @param ctx NOT USED
-    /// @return beforeData NOT USED
-    function beforeAgreementCreated(
-        ISuperToken superToken,
-        address agreementClass,
-        bytes32 agreementId,
-        bytes calldata agreementData,
-        bytes calldata ctx
-    ) external pure override returns (bytes memory beforeData) {
+    function beforeAgreementCreated(ISuperToken, address, bytes32, bytes calldata, bytes calldata)
+        external
+        pure
+        override
+        returns (bytes memory beforeData)
+    {
         return "0x";
     }
 
     /// @dev This callback is called after the flow is created
     /// @param superToken The super token
     /// @param agreementClass The agreement class
-    /// @param agreementId NOT USED
     /// @param agreementData The agreement data
-    /// @param cbdata NOT USED
     /// @param ctx The callback context
     /// @return newCtx The new callback context
     function afterAgreementCreated(
         ISuperToken superToken,
         address agreementClass,
-        bytes32 agreementId,
+        bytes32,
         bytes calldata agreementData,
-        bytes calldata cbdata,
+        bytes calldata,
         bytes calldata ctx
     ) external override returns (bytes memory newCtx) {
         _checkHookParam(superToken);
@@ -240,17 +231,14 @@ contract RecipientSuperApp is ISuperApp {
     /// @dev This callback is called before the flow is updated
     /// @param superToken The super token
     /// @param agreementClass The agreement class
-    /// @param agreementId NOT USED
     /// @param agreementData The agreement data
-    /// @param ctx NOT USED
-    /// @return beforeData NOT USED
     function beforeAgreementUpdated(
         ISuperToken superToken,
         address agreementClass,
-        bytes32 agreementId,
+        bytes32,
         bytes calldata agreementData,
-        bytes calldata ctx
-    ) external view override returns (bytes memory beforeData) {
+        bytes calldata
+    ) external view override returns (bytes memory) {
         _checkHookParam(superToken);
         if (!isAcceptedAgreement(agreementClass)) return "0x";
 
@@ -260,15 +248,14 @@ contract RecipientSuperApp is ISuperApp {
     /// @dev This callback is called after the flow is updated
     /// @param superToken The super token
     /// @param agreementClass The agreement class
-    /// @param agreementId NOT USED
     /// @param agreementData The agreement data
     /// @param cbdata The callback data
     /// @param ctx The callback context
     /// @return The new callback context
     function afterAgreementUpdated(
         ISuperToken superToken,
         address agreementClass,
-        bytes32 agreementId,
+        bytes32,
         bytes calldata agreementData,
         bytes calldata cbdata,
         bytes calldata ctx
@@ -294,17 +281,14 @@ contract RecipientSuperApp is ISuperApp {
     /// @dev This callback is called before the flow is terminated
     /// @param superToken The super token
     /// @param agreementClass The agreement class
-    /// @param agreementId NOT USED
     /// @param agreementData The agreement data
-    /// @param ctx NOT USED
-    /// @return beforeData NOT USED
     function beforeAgreementTerminated(
         ISuperToken superToken,
         address agreementClass,
-        bytes32 agreementId,
+        bytes32,
         bytes calldata agreementData,
-        bytes calldata ctx
-    ) external view override returns (bytes memory beforeData) {
+        bytes calldata
+    ) external view override returns (bytes memory) {
         if (msg.sender != address(HOST) || !isAcceptedAgreement(agreementClass) || !isAcceptedSuperToken(superToken)) {
             return "0x";
         }
@@ -315,16 +299,14 @@ contract RecipientSuperApp is ISuperApp {
     /// @dev This callback is called after the flow is terminated
     /// @param superToken The super token
     /// @param agreementClass The agreement class
-    /// @param agreementId NOT USED
-    /// @param agreementData NOT USED
     /// @param cbdata The callback data
     /// @param ctx The callback context
     /// @return The new callback context
     function afterAgreementTerminated(
         ISuperToken superToken,
         address agreementClass,
-        bytes32 agreementId,
-        bytes calldata agreementData,
+        bytes32,
+        bytes calldata,
         bytes calldata cbdata,
         bytes calldata ctx
     ) external override returns (bytes memory) {