refactor(contracts): add missing harnesses and comments to base and advanced test contracts

Author: 0xjei

packages/contracts/contracts/src/test/advanced/AdvancedERC721Checker.sol

@@ -5,18 +5,14 @@ import {AdvancedChecker} from "../../core/checker/AdvancedChecker.sol";
 import {BaseERC721Checker} from "../base/BaseERC721Checker.sol";
 import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
 
-/// @title AdvancedERC721Checker.
-/// @notice Multi-phase NFT validation with aggregated verification contracts.
-/// @dev Implements three-phase validation using multiple NFT contracts and external verifiers:
-///      - Pre-check: Basic signup token validation using BaseERC721Checker.
-///      - Main-check: Balance threshold validation for signup token.
-///      - Post-check: Reward eligibility verification for reward token.
+/// @title AdvancedERC721Checker
+/// @notice Multi-phase NFT validation using external contracts and thresholds.
+/// @dev Implements three-phase validation:
+///      - Pre-check: Basic ownership verification using `BaseERC721Checker`.
+///      - Main-check: Ensures a minimum token balance.
+///      - Post-check: Validates reward eligibility.
 contract AdvancedERC721Checker is AdvancedChecker {
-    /// @notice External contracts used for verification.
-    /// @dev Immutable references derived from verifier array positions:
-    ///      - Index 0: Signup NFT contract.
-    ///      - Index 1: Reward NFT contract.
-    ///      - Index 2: Base ERC721 checker contract.
+    /// @notice External verification contracts and thresholds.
     IERC721 public signupNft;
     IERC721 public rewardNft;
     BaseERC721Checker public baseERC721Checker;
@@ -25,14 +21,13 @@ contract AdvancedERC721Checker is AdvancedChecker {
     uint256 public minTokenId;
     uint256 public maxTokenId;
 
+    /// @notice Initializes the checker with external contract references and thresholds.
+    /// @dev Decodes appended bytes to set state variables.
     function _initialize() internal override {
-        // 1. Call super to handle `_initialized` check.
         super._initialize();
 
-        // 2. Retrieve appended bytes from the clone.
         bytes memory data = _getAppendedBytes();
 
-        // 3. Decode everything in one shot:
         (
             address signupNftAddr,
             address rewardNftAddr,
@@ -42,7 +37,6 @@ contract AdvancedERC721Checker is AdvancedChecker {
             uint256 maxTokenId_
         ) = abi.decode(data, (address, address, address, uint256, uint256, uint256));
 
-        // 4. Assign to storage variables.
         signupNft = IERC721(signupNftAddr);
         rewardNft = IERC721(rewardNftAddr);
         baseERC721Checker = BaseERC721Checker(baseCheckerAddr);
@@ -51,33 +45,33 @@ contract AdvancedERC721Checker is AdvancedChecker {
         maxTokenId = maxTokenId_;
     }
 
-    /// @notice Pre-check: Validates initial NFT ownership.
-    /// @dev Delegates basic ownership check to BaseERC721Checker.
+    /// @notice Pre-check: Validates ownership using the base checker.
     /// @param subject Address to validate.
-    /// @param evidence Array containing encoded tokenId.
-    /// @return Validation status from base checker.
+    /// @param evidence Encoded tokenId.
+    /// @return Boolean indicating validation success.
     function _checkPre(address subject, bytes[] calldata evidence) internal view override returns (bool) {
         super._checkPre(subject, evidence);
+
         return baseERC721Checker.check(subject, evidence);
     }
 
-    /// @notice Main-check: Validates token balance requirements.
-    /// @dev Ensures subject has exactly MIN_BALANCE tokens.
+    /// @notice Main-check: Ensures token balance meets requirements.
     /// @param subject Address to validate.
-    /// @param evidence Not used in balance check.
-    /// @return True if balance meets requirements.
+    /// @param evidence Not used in this validation.
+    /// @return Boolean indicating validation success.
     function _checkMain(address subject, bytes[] calldata evidence) internal view override returns (bool) {
         super._checkMain(subject, evidence);
-        return signupNft.balanceOf(subject) >= minBalance && signupNft.balanceOf(subject) <= minBalance;
+
+        return signupNft.balanceOf(subject) >= minBalance;
     }
 
     /// @notice Post-check: Validates reward eligibility.
-    /// @dev Ensures subject doesn't already have reward tokens.
     /// @param subject Address to validate.
-    /// @param evidence Not used in reward check.
-    /// @return True if subject eligible for rewards.
+    /// @param evidence Not used in this validation.
+    /// @return Boolean indicating validation success.
     function _checkPost(address subject, bytes[] calldata evidence) internal view override returns (bool) {
         super._checkPost(subject, evidence);
+
         return rewardNft.balanceOf(subject) == 0;
     }
 }

packages/contracts/contracts/src/test/advanced/AdvancedERC721CheckerFactory.sol

@@ -2,12 +2,23 @@
 pragma solidity ^0.8.20;
 
 import {AdvancedERC721Checker} from "./AdvancedERC721Checker.sol";
-import {LibClone} from "solady/src/utils/LibClone.sol";
 import {Factory} from "../../core/proxy/Factory.sol";
 
+/// @title AdvancedERC721CheckerFactory
+/// @notice Factory for deploying minimal proxy instances of AdvancedERC721Checker.
+/// @dev Encodes configuration data for each clone.
 contract AdvancedERC721CheckerFactory is Factory {
+    /// @notice Initializes the factory with the AdvancedERC721Checker implementation.
     constructor() Factory(address(new AdvancedERC721Checker())) {}
 
+    /// @notice Deploys a new AdvancedERC721Checker clone.
+    /// @dev Encodes and appends configuration data for the clone.
+    /// @param _nftAddress Address of the signup NFT contract.
+    /// @param _rewardNft Address of the reward NFT contract.
+    /// @param _baseERC721Checker Address of the base checker contract.
+    /// @param _minBalance Minimum balance required for validation.
+    /// @param _minTokenId Minimum token ID for validation.
+    /// @param _maxTokenId Maximum token ID for validation.
     function deploy(
         address _nftAddress,
         address _rewardNft,
@@ -16,7 +27,6 @@ contract AdvancedERC721CheckerFactory is Factory {
         uint256 _minTokenId,
         uint256 _maxTokenId
     ) public {
-        // 1. Encode.
         bytes memory data = abi.encode(
             _nftAddress,
             _rewardNft,
@@ -26,10 +36,8 @@ contract AdvancedERC721CheckerFactory is Factory {
             _maxTokenId
         );
 
-        // 2. Deploy.
         address clone = super._deploy(data);
 
-        // 3. Call initialize().
         AdvancedERC721Checker(clone).initialize();
     }
 }

packages/contracts/contracts/src/test/advanced/AdvancedERC721Policy.sol

@@ -2,13 +2,13 @@
 pragma solidity ^0.8.20;
 
 import {AdvancedPolicy} from "../../core/policy/AdvancedPolicy.sol";
-import {AdvancedERC721Checker} from "./AdvancedERC721Checker.sol";
 
-/// @title AdvancedERC721Policy.
-/// @notice Three-phase ERC721 validation policy.
-/// @dev Enforces multi-stage checks through AdvancedERC721Checker.
+/// @title AdvancedERC721Policy
+/// @notice Three-phase policy contract for ERC721 validation.
+/// @dev Leverages AdvancedChecker for pre, main, and post validation phases.
 contract AdvancedERC721Policy is AdvancedPolicy {
-    /// @notice Returns policy identifier.
+    /// @notice Returns a unique identifier for the policy.
+    /// @return The string identifier "AdvancedERC721".
     function trait() external pure returns (string memory) {
         return "AdvancedERC721";
     }

packages/contracts/contracts/src/test/advanced/AdvancedERC721PolicyFactory.sol

@@ -1,25 +1,26 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.20;
 
-import {LibClone} from "solady/src/utils/LibClone.sol";
 import {AdvancedERC721Policy} from "./AdvancedERC721Policy.sol";
 import {Factory} from "../../core/proxy/Factory.sol";
 
-/**
- * @title AdvancedERC721PolicyFactory
- * @notice Example factory for deploying minimal proxies of `AdvancedERC721Policy`.
- */
+/// @title AdvancedERC721PolicyFactory
+/// @notice Factory for deploying minimal proxy instances of AdvancedERC721Policy.
+/// @dev Encodes configuration data for multi-phase policy validation.
 contract AdvancedERC721PolicyFactory is Factory {
+    /// @notice Initializes the factory with the AdvancedERC721Policy implementation.
     constructor() Factory(address(new AdvancedERC721Policy())) {}
 
+    /// @notice Deploys a new AdvancedERC721Policy clone.
+    /// @param _checkerAddr Address of the associated checker contract.
+    /// @param _skipPre Whether to skip pre-checks.
+    /// @param _skipPost Whether to skip post-checks.
+    /// @param _allowMultipleMain Whether multiple main checks are allowed.
     function deploy(address _checkerAddr, bool _skipPre, bool _skipPost, bool _allowMultipleMain) public {
-        // 1. Encode.
         bytes memory data = abi.encode(msg.sender, _checkerAddr, _skipPre, _skipPost, _allowMultipleMain);
 
-        // 2. Deploy.
         address clone = super._deploy(data);
 
-        // 3. Call `initialize()`.
         AdvancedERC721Policy(clone).initialize();
     }
 }

packages/contracts/contracts/src/test/advanced/AdvancedVoting.sol

@@ -4,102 +4,113 @@ pragma solidity ^0.8.20;
 import {AdvancedPolicy} from "../../core/policy/AdvancedPolicy.sol";
 import {Check} from "../../core/interfaces/IAdvancedPolicy.sol";
 
-/// @title AdvancedVoting.
-/// @notice Advanced voting system with NFT-based phases and eligibility verification.
-/// @dev Implements a three-phase governance process using NFT validation:
-///      1. Registration: Validates ownership of signup NFT (under-the-hood uses the BaseERC721Checker).
-///      2. Voting: Validates token balances and records votes (single token = single vote).
-///      3. Eligibility: Validates criteria for governance participation benefits.
+/// @title AdvancedVoting
+/// @notice Multi-phase governance system with NFT-based validation.
+/// @dev Combines pre, main, and post phases for registration, voting, and eligibility verification.
 contract AdvancedVoting {
-    /// @notice Emitted on successful phase completion.
-    /// @param voter Address that completed the phase.
+    /// @notice Emitted when a voter registers successfully.
+    /// @param voter Address of the voter who registered.
     event Registered(address voter);
-    /// @param option Selected voting option (0 or 1).
+
+    /// @notice Emitted when a vote is cast successfully.
+    /// @param voter Address of the voter who cast their vote.
+    /// @param option The chosen voting option (0 or 1).
     event Voted(address voter, uint8 option);
-    /// @param voter Address that met eligibility criteria.
+
+    /// @notice Emitted when a voter is deemed eligible.
+    /// @param voter Address of the voter who met eligibility criteria.
     event Eligible(address voter);
 
-    /// @notice Validation error states.
-    /// @dev Thrown when phase requirements not met.
-    error NotRegistered(); // Pre-check (registration) not completed.
-    error NotVoted(); // Main check (voting) not completed.
-    error AlreadyEligible(); // Post check (eligibility) already verified.
-    error InvalidOption(); // Vote option out of valid range.
-    error NotEligible(); // Eligibility criteria not met.
+    /// @notice Error thrown when a user attempts an action without registering first.
+    error NotRegistered();
+
+    /// @notice Error thrown when a user attempts to verify eligibility without voting.
+    error NotVoted();
+
+    /// @notice Error thrown when a user tries to verify eligibility more than once.
+    error AlreadyEligible();
+
+    /// @notice Error thrown when an invalid voting option is provided.
+    error InvalidOption();
 
-    /// @notice Policy contract managing multi-phase validation.
-    /// @dev Handles all NFT-based checks through aggregated verifiers.
+    /// @notice Error thrown when a user does not meet the eligibility criteria.
+    error NotEligible();
+
+    /// @notice Reference to the policy contract enforcing multi-phase validation.
     AdvancedPolicy public immutable POLICY;
 
-    /// @notice Vote tracking per option.
-    /// @dev Maps option ID (0 or 1) to total votes received.
+    /// @notice Tracks the vote count for each option (0 or 1).
     mapping(uint8 => uint256) public voteCounts;
 
-    /// @notice Initializes voting system.
-    /// @param _policy Advanced policy contract with configured verifiers.
+    /// @notice Constructor to set the policy contract.
+    /// @param _policy Address of the AdvancedPolicy contract to use for validation.
     constructor(AdvancedPolicy _policy) {
         POLICY = _policy;
     }
 
-    /// @notice Registration phase handler.
-    /// @dev Validates signup NFT ownership using BaseERC721Checker.
-    /// @param tokenId ID of the signup NFT to validate.
-    /// @custom:requirements
-    /// - Token must exist.
-    /// - Caller must be token owner.
-    /// - Token ID must be within valid range.
-    /// @custom:emits Registered when registration succeeds.
+    /// @notice Registers a user for voting by validating their NFT ownership.
+    /// @dev Enforces the pre-check phase using the AdvancedPolicy contract.
+    /// @param tokenId The ID of the NFT used to verify registration eligibility.
     function register(uint256 tokenId) external {
+        // Prepare evidence with the tokenId encoded as bytes.
         bytes[] memory _evidence = new bytes[](1);
         _evidence[0] = abi.encode(tokenId);
 
+        // Enforce the pre-check phase using the provided policy.
         POLICY.enforce(msg.sender, _evidence, Check.PRE);
 
+        // Emit an event to log the registration.
         emit Registered(msg.sender);
     }
 
-    /// @notice Voting phase handler.
-    /// @dev Validates voting power and records vote choice.
-    /// @param option Binary choice (0 or 1).
-    /// @custom:requirements
-    /// - Registration must be completed.
-    /// - Option must be valid (0 or 1).
-    /// - Token balance must meet requirements.
-    /// @custom:emits Voted when vote is recorded.
+    /// @notice Allows a registered user to cast their vote.
+    /// @dev Enforces the main-check phase and updates the vote count.
+    /// @param option The chosen voting option (0 or 1).
     function vote(uint8 option) external {
+        // Retrieve the enforcement status of the sender from the policy.
         (bool pre, , ) = POLICY.enforced(msg.sender);
+
+        // Ensure the user has registered before voting.
         if (!pre) revert NotRegistered();
+
+        // Validate that the voting option is within the allowed range.
         if (option >= 2) revert InvalidOption();
 
+        // Prepare evidence with the chosen option encoded as bytes.
         bytes[] memory _evidence = new bytes[](1);
         _evidence[0] = abi.encode(option);
 
+        // Enforce the main-check phase using the policy.
         POLICY.enforce(msg.sender, _evidence, Check.MAIN);
 
+        // Increment the vote count for the chosen option.
         unchecked {
             voteCounts[option]++;
         }
 
+        // Emit an event to log the voting action.
         emit Voted(msg.sender, option);
     }
 
-    /// @notice Eligibility verification phase.
-    /// @dev Validates completion of governance process and checks eligibility criteria.
-    /// @custom:requirements
-    /// - Caller must be registered (passed PRE check).
-    /// - Caller must have voted (passed MAIN check).
-    /// - Caller must not be already verified (no POST check).
-    /// - Caller must meet eligibility criteria (no existing benefits).
-    /// @custom:emits Eligible when verification succeeds.
+    /// @notice Verifies a user's eligibility after voting has concluded.
+    /// @dev Enforces the post-check phase to ensure eligibility criteria are met.
     function eligible() external {
+        // Retrieve the enforcement status for all phases.
         (bool pre, uint8 main, bool post) = POLICY.enforced(msg.sender);
 
+        // Ensure the user has completed the registration phase.
         if (!pre) revert NotRegistered();
+
+        // Ensure the user has cast at least one vote.
         if (main == 0) revert NotVoted();
+
+        // Ensure the user has not already been marked as eligible.
         if (post) revert AlreadyEligible();
 
+        // Enforce the post-check phase using the policy.
         POLICY.enforce(msg.sender, new bytes[](1), Check.POST);
 
+        // Emit an event to log the eligibility status.
         emit Eligible(msg.sender);
     }
 }