feat(cleanup): imports, natspec, init flow, params, etc

README.md

@@ -164,8 +164,7 @@ The parameters are different depending on the type of Cookie Jar being created.
 | 1         | uint256 | The length of the period for the Cookie Jar.               |
 | 2         | uint256 | The amount of cookies to be distributed per period.        |
 | 3         | address | The address of the cookie token contract.                  |
-| 4         | address | The address of the Hats contract.                          |
-| 5         | uint256 | The HatID to check eligibility against                     |
+| 4         | uint256 | The HatID to check eligibility against                     |
 
 #### Mapping Cookie Jar
 

src/ERC6551/BaalCookieJar6551.sol

@@ -5,12 +5,8 @@ import { BaalAllowlist } from "src/core/allowlists/BaalAllowlist.sol";
 import { CookieJar6551 } from "src/ERC6551/CookieJar6551.sol";
 
 contract BaalCookieJar6551 is BaalAllowlist, CookieJar6551 {
-    function setUp(bytes memory _initializationParams) public override(BaalAllowlist, CookieJar6551) {
-        BaalAllowlist.setUp(_initializationParams);
-        CookieJar6551.setUp(_initializationParams);
-    }
-
-    function isAllowList(address user) public view override returns (bool) {
-        return BaalAllowlist._isAllowList(user);
+    function setUp(bytes memory _initializationParams) public override {
+        __Allowlist_init(_initializationParams);
+        super.setUp(_initializationParams);
     }
 }

src/ERC6551/CookieJar6551.sol

@@ -5,24 +5,20 @@ import { CookieJarCore } from "src/core/CookieJarCore.sol";
 import { Giver6551 } from "src/core/givers/Giver6551.sol";
 import { CookieUtils } from "src/lib/CookieUtils.sol";
 
-contract CookieJar6551 is CookieJarCore, Giver6551 {
-    function setUp(bytes memory _initializationParams) public virtual override(CookieJarCore) initializer {
-        (address _target) = abi.decode(_initializationParams, (address));
-        target = _target;
-
+/**
+ * @title CookieJar6551
+ * @dev This contract extends Giver6551 and CookieJarCore to provide functionality specific to ERC6551.
+ */
+abstract contract CookieJar6551 is Giver6551 {
+    /**
+     * @notice Sets up the contract with the provided initialization parameters.
+     * @dev This function decodes the initialization parameters, sets the target state variable, calls the parent setUp
+     * function, and transfers ownership to the target.
+     * @param _initializationParams The parameters to initialize the contract. It should be encoded as an address.
+     */
+    function setUp(bytes memory _initializationParams) public virtual override {
         super.setUp(_initializationParams);
 
-        transferOwnership(_target);
-    }
-
-    function giveCookie(
-        address cookieMonster,
-        uint256 amount
-    )
-        internal
-        override(CookieJarCore)
-        returns (bytes32 cookieUid)
-    {
-        cookieUid = Giver6551.giveCookie(cookieMonster, amount, cookieToken);
+        transferOwnership(target);
     }
 }

src/ERC6551/ERC20CookieJar6551.sol

@@ -5,13 +5,8 @@ import { ERC20Allowlist } from "src/core/allowlists/ERC20Allowlist.sol";
 import { CookieJar6551 } from "src/ERC6551/CookieJar6551.sol";
 
 contract ERC20CookieJar6551 is ERC20Allowlist, CookieJar6551 {
-    function setUp(bytes memory _initializationParams) public override(ERC20Allowlist, CookieJar6551) {
-        ERC20Allowlist.setUp(_initializationParams);
-
-        CookieJar6551.setUp(_initializationParams);
-    }
-
-    function isAllowList(address user) public view override returns (bool) {
-        return ERC20Allowlist._isAllowList(user);
+    function setUp(bytes memory _initializationParams) public override {
+        __Allowlist_init(_initializationParams);
+        super.setUp(_initializationParams);
     }
 }

src/ERC6551/ERC721CookieJar6551.sol

@@ -5,10 +5,9 @@ import { ERC721Allowlist } from "src/core/allowlists/ERC721Allowlist.sol";
 import { CookieJar6551 } from "src/ERC6551/CookieJar6551.sol";
 
 contract ERC721CookieJar6551 is ERC721Allowlist, CookieJar6551 {
-    function setUp(bytes memory _initializationParams) public override(ERC721Allowlist, CookieJar6551) {
-        ERC721Allowlist.setUp(_initializationParams);
-
-        CookieJar6551.setUp(_initializationParams);
+    function setUp(bytes memory _initializationParams) public override {
+        __Allowlist_init(_initializationParams);
+        super.setUp(_initializationParams);
     }
 
     function isAllowList(address user) public view override returns (bool) {

src/ERC6551/HatsCookieJar6551.sol

@@ -5,12 +5,8 @@ import { HatsAllowlist } from "src/core/allowlists/HatsAllowlist.sol";
 import { CookieJar6551 } from "src/ERC6551/CookieJar6551.sol";
 
 contract HatsCookieJar6551 is HatsAllowlist, CookieJar6551 {
-    function setUp(bytes memory _initializationParams) public override(HatsAllowlist, CookieJar6551) {
-        HatsAllowlist.setUp(_initializationParams);
-        CookieJar6551.setUp(_initializationParams);
-    }
-
-    function isAllowList(address user) public view override returns (bool) {
-        return HatsAllowlist._isAllowList(user);
+    function setUp(bytes memory _initializationParams) public override {
+        __Allowlist_init(_initializationParams);
+        super.setUp(_initializationParams);
     }
 }

src/ERC6551/ListCookieJar6551.sol

@@ -5,21 +5,16 @@ import { MappedAllowlist } from "src/core/allowlists/MappedAllowlist.sol";
 import { CookieJar6551 } from "src/ERC6551/CookieJar6551.sol";
 
 contract ListCookieJar6551 is MappedAllowlist, CookieJar6551 {
-    function setUp(bytes memory _initializationParams) public override(MappedAllowlist, CookieJar6551) {
-        MappedAllowlist.setUp(_initializationParams);
-
-        CookieJar6551.setUp(_initializationParams);
-    }
-
-    function isAllowList(address user) public view override returns (bool) {
-        return MappedAllowlist._isAllowList(user);
+    function setUp(bytes memory _initializationParams) public override {
+        __Allowlist_init(_initializationParams);
+        super.setUp(_initializationParams);
     }
 
     function setAllowList(address _address, bool _isAllowed) external onlyOwner {
-        allowList[_address] = _isAllowed;
+        _setAllowList(_address, _isAllowed);
     }
 
     function batchAllowList(address[] memory _addresses, bool[] memory _isAllowed) external onlyOwner {
-        MappedAllowlist._batchAllowList(_addresses, _isAllowed);
+        _batchAllowList(_addresses, _isAllowed);
     }
 }

src/ERC6551/OpenCookieJar6551.sol

@@ -2,9 +2,11 @@
 pragma solidity 0.8.19;
 
 import { CookieJar6551 } from "src/ERC6551/CookieJar6551.sol";
+import { OpenAllowlist } from "../core/allowlists/OpenAllowlist.sol";
 
-contract OpenCookieJar6551 is CookieJar6551 {
+contract OpenCookieJar6551 is OpenAllowlist, CookieJar6551 {
     function setUp(bytes memory _initializationParams) public override {
-        CookieJar6551.setUp(_initializationParams);
+        __Allowlist_init(_initializationParams);
+        super.setUp(_initializationParams);
     }
 }

src/ERC6551/erc6551/ERC6551Module.sol

@@ -8,11 +8,11 @@ import "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
 import "@openzeppelin/contracts/utils/introspection/IERC165.sol";
 import "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol";
 
-import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
+import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 
 import "src/lib/MinimalReceiver.sol";
-import {IAccount} from "src/interfaces/IERC6551.sol";
-import {ERC6551AccountLib} from "src/lib/ERC6551AccountLib.sol";
+import { IAccount } from "src/interfaces/IERC6551.sol";
+import { ERC6551AccountLib } from "src/lib/ERC6551AccountLib.sol";
 
 /**
  * @title A smart contract wallet owned by a single ERC721 token
@@ -43,7 +43,7 @@ contract AccountERC6551 is IERC165, IERC1271, IAccount, MinimalReceiver, Initial
      */
     event ExecutorUpdated(address owner, address executor);
 
-    constructor() {}
+    constructor() { }
 
     /**
      * @dev Ensures execution can only continue if the account is not locked
@@ -73,7 +73,11 @@ contract AccountERC6551 is IERC165, IERC1271, IAccount, MinimalReceiver, Initial
      * @param value   Ether value of the transaction
      * @param data    Encoded payload of the transaction
      */
-    function executeCall(address to, uint256 value, bytes calldata data)
+    function executeCall(
+        address to,
+        uint256 value,
+        bytes calldata data
+    )
         external
         payable
         onlyUnlocked
@@ -92,7 +96,11 @@ contract AccountERC6551 is IERC165, IERC1271, IAccount, MinimalReceiver, Initial
      * @param value   Ether value of the transaction
      * @param data    Encoded payload of the transaction
      */
-    function exec(address to, uint256 value, bytes calldata data)
+    function exec(
+        address to,
+        uint256 value,
+        bytes calldata data
+    )
         external
         payable
         onlyUnlocked
@@ -111,7 +119,11 @@ contract AccountERC6551 is IERC165, IERC1271, IAccount, MinimalReceiver, Initial
      * @param value   Ether value of the transaction
      * @param data    Encoded payload of the transaction
      */
-    function executeTrustedCall(address to, uint256 value, bytes calldata data)
+    function executeTrustedCall(
+        address to,
+        uint256 value,
+        bytes calldata data
+    )
         external
         payable
         onlyUnlocked
@@ -296,7 +308,7 @@ contract AccountERC6551 is IERC165, IERC1271, IAccount, MinimalReceiver, Initial
      */
     function _call(address to, uint256 value, bytes calldata data) internal returns (bytes memory result) {
         bool success;
-        (success, result) = to.call{value: value}(data);
+        (success, result) = to.call{ value: value }(data);
 
         if (!success) {
             assembly {

src/ERC6551/erc6551/ERC6551Registry.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.19;
 
-import {Create2} from "@openzeppelin/contracts/utils/Create2.sol";
+import { Create2 } from "@openzeppelin/contracts/utils/Create2.sol";
 
 import "src/lib/ERC6551BytecodeLib.sol";
 
@@ -14,7 +14,6 @@ import "src/ERC6551/erc6551/ERC6551Registry.sol";
  * @dev Determines the address for each token bound account and performs deployment of accounts
  * @author https://github.com/ethereum/EIPs/pull/6551/files
  */
-
 contract AccountRegistry is IRegistry {
     error InitializationFailed();
 
@@ -25,7 +24,10 @@ contract AccountRegistry is IRegistry {
         uint256 tokenId,
         uint256 salt,
         bytes calldata initData
-    ) external returns (address) {
+    )
+        external
+        returns (address)
+    {
         bytes memory code = ERC6551BytecodeLib.getCreationCode(implementation, chainId, tokenContract, tokenId, salt);
 
         address _account = Create2.computeAddress(bytes32(salt), keccak256(code));
@@ -44,7 +46,13 @@ contract AccountRegistry is IRegistry {
         return _account;
     }
 
-    function account(address implementation, uint256 chainId, address tokenContract, uint256 tokenId, uint256 salt)
+    function account(
+        address implementation,
+        uint256 chainId,
+        address tokenContract,
+        uint256 tokenId,
+        uint256 salt
+    )
         external
         view
         returns (address)

src/ERC6551/nft/CookieNFT.sol

@@ -17,6 +17,8 @@ import { IAccount } from "src/interfaces/IERC6551.sol";
 import { MinimalReceiver } from "src/lib/MinimalReceiver.sol";
 import { Base64 } from "src/lib/Base64.sol";
 
+import { NOT_APPROVED_OR_OWNER, INVALID_TOKEN_ID } from "src/lib/Errors.sol";
+
 contract CookieNFT is ERC721 {
     using Counters for Counters.Counter;
 
@@ -180,12 +182,16 @@ contract CookieNFT is ERC721 {
      * param _tokenId the token ID
      */
     function tokenURI(uint256 _tokenId) public view override returns (string memory) {
-        require(_exists(_tokenId), "ERC721Metadata: URI query for nonexistent token");
+        if (!_exists(_tokenId)) {
+            revert INVALID_TOKEN_ID(_tokenId);
+        }
         return string(_constructTokenURI(_tokenId));
     }
 
     function burn(uint256 _tokenId) public {
-        require(_isApprovedOrOwner(_msgSender(), _tokenId), "ERC721: caller is not token owner or approved");
+        if (!_isApprovedOrOwner(_msgSender(), _tokenId)) {
+            revert NOT_APPROVED_OR_OWNER();
+        }
         _burn(_tokenId);
     }
 }

src/SafeModule/ZodiacBaalCookieJar.sol

@@ -4,13 +4,21 @@ pragma solidity 0.8.19;
 import { BaalAllowlist } from "src/core/allowlists/BaalAllowlist.sol";
 import { ZodiacCookieJar } from "./ZodiacCookieJar.sol";
 
+/**
+ * @title ZodiacBaalCookieJar
+ * @dev This contract extends BaalAllowlist and ZodiacCookieJar to provide a mechanism for maintaining a cookie jar with
+ * an allowlist.
+ */
 contract ZodiacBaalCookieJar is BaalAllowlist, ZodiacCookieJar {
-    function setUp(bytes memory _initializationParams) public override(BaalAllowlist, ZodiacCookieJar) {
-        BaalAllowlist.setUp(_initializationParams);
-        ZodiacCookieJar.setUp(_initializationParams);
-    }
-
-    function isAllowList(address user) public view override returns (bool) {
-        return BaalAllowlist._isAllowList(user);
+    /**
+     * @notice Sets up the ZodiacBaalCookieJar contract.
+     * @dev This function is public and overrides the base contracts' setup functions.
+     * It first calls the __Allowlist_init function with the provided initialization parameters, then calls the base
+     * contracts' setup functions.
+     * @param _initializationParams The initialization parameters as bytes.
+     */
+    function setUp(bytes memory _initializationParams) public override {
+        __Allowlist_init(_initializationParams);
+        super.setUp(_initializationParams);
     }
 }

src/SafeModule/ZodiacCookieJar.sol

@@ -1,51 +1,25 @@
 // SPDX-License-Identifier: Unlicense
 pragma solidity 0.8.19;
 
-import { Module } from "@gnosis.pm/zodiac/contracts/core/Module.sol";
-import { Enum } from "@gnosis.pm/safe-contracts/contracts/common/Enum.sol";
-import { Strings } from "@openzeppelin/contracts/utils/Strings.sol";
-
 import { CookieJarCore } from "src/core/CookieJarCore.sol";
 import { GiverZodiac } from "src/core/givers/GiverZodiac.sol";
-import { IPoster } from "@daohaus/baal-contracts/contracts/interfaces/IPoster.sol";
-import { FactoryFriendly } from "@gnosis.pm/zodiac/contracts/factory/FactoryFriendly.sol";
 import { CookieUtils } from "src/lib/CookieUtils.sol";
 
-contract ZodiacCookieJar is CookieJarCore, GiverZodiac {
+/**
+ * @title ZodiacCookieJar
+ * @dev This abstract contract extends GiverZodiac to provide a base for contracts that manage a cookie jar.
+ */
+abstract contract ZodiacCookieJar is GiverZodiac {
     /**
-     * @notice Sets up the contract with the given initialization parameters.
-     * @dev The initialization parameters are decoded from a bytes array into the Safe target, period length, cookie
-     * amount, and cookie token.
-     * The Safe target is set as both the avatar and target for the module.  This means that the module cannot be
-     * chained in a series of modules.
-     * A check is done to ensure the cookie amount is greater than the percentage points constant.
-     * The period length, cookie amount, and cookie token are then set as per the parameters.
-     * An event is emitted with the initialization parameters.
-     * @param _initializationParams The initialization parameters, encoded as a bytes array.
+     * @notice Sets up the ZodiacCookieJar contract.
+     * @dev This function is public and virtual, and it overrides the base contract's setup function.
+     * It first calls the base contract's setUp function with the provided initialization parameters, then transfers the
+     * ownership to the target.
+     * @param _initializationParams The initialization parameters as bytes.
      */
-    function setUp(bytes memory _initializationParams)
-        public
-        virtual
-        override(CookieJarCore, FactoryFriendly)
-        initializer
-    {
-        (address _safeTarget) = abi.decode(_initializationParams, (address));
+    function setUp(bytes memory _initializationParams) public virtual override {
         super.setUp(_initializationParams);
 
-        setAvatar(_safeTarget);
-        setTarget(_safeTarget);
-
-        transferOwnership(_safeTarget);
-    }
-
-    /**
-     * @notice Transfers the specified amount of cookies to a given address.
-     * @dev Calculates the sustainability fee and deducts it from the amount. Then, depending on whether the cookie is
-     * an ERC20 token or ether, it executes the transfer operation. Finally, it emits a GiveCookie event.
-     * @param cookieMonster The address to receive the cookie.
-     * @param amount The amount of cookie to be transferred.
-     */
-    function giveCookie(address cookieMonster, uint256 amount) internal override returns (bytes32 cookieUid) {
-        cookieUid = GiverZodiac.giveCookie(cookieMonster, amount, cookieToken);
+        transferOwnership(target);
     }
 }