From c74c32d27c6ca42708e1c13d1f3c3087d971bfd0 Mon Sep 17 00:00:00 2001 From: Giacomo Date: Mon, 2 Dec 2024 17:00:54 +0100 Subject: [PATCH] refactor(contracts): improve contracts and test documentation; small code optimization and refactor --- .../contracts/src/AdvancedChecker.sol | 91 ++++--- .../contracts/src/AdvancedPolicy.sol | 106 +++++---- .../contracts/contracts/src/BaseChecker.sol | 25 +- .../contracts/contracts/src/BasePolicy.sol | 38 ++- packages/contracts/contracts/src/Policy.sol | 32 ++- .../src/interfaces/IAdvancedChecker.sol | 28 ++- .../src/interfaces/IAdvancedPolicy.sol | 32 ++- .../contracts/src/interfaces/IBaseChecker.sol | 12 +- .../contracts/src/interfaces/IBasePolicy.sol | 19 +- .../contracts/src/interfaces/IPolicy.sol | 29 +-- .../contracts/src/test/Advanced.t.sol | 164 ++++++------- .../contracts/contracts/src/test/Base.t.sol | 62 ++--- .../test/advanced/AdvancedERC721Checker.sol | 59 ++--- .../test/advanced/AdvancedERC721Policy.sol | 15 +- .../src/test/advanced/AdvancedVoting.sol | 62 +++-- .../src/test/base/BaseERC721Checker.sol | 27 +-- .../src/test/base/BaseERC721Policy.sol | 15 +- .../contracts/src/test/base/BaseVoting.sol | 44 +++- .../contracts/src/test/utils/NFT.sol | 6 + .../wrappers/AdvancedERC721CheckerHarness.sol | 43 ++-- .../wrappers/AdvancedERC721PolicyHarness.sol | 13 +- .../wrappers/BaseERC721CheckerHarness.sol | 12 +- .../test/wrappers/BaseERC721PolicyHarness.sol | 11 +- packages/contracts/test/Advanced.test.ts | 222 +++++++++--------- packages/contracts/test/Base.test.ts | 92 ++++---- 25 files changed, 666 insertions(+), 593 deletions(-) diff --git a/packages/contracts/contracts/src/AdvancedChecker.sol b/packages/contracts/contracts/src/AdvancedChecker.sol index 9693ae7..d045aa6 100644 --- a/packages/contracts/contracts/src/AdvancedChecker.sol +++ b/packages/contracts/contracts/src/AdvancedChecker.sol @@ -3,41 +3,45 @@ pragma solidity 0.8.27; import {IAdvancedChecker, Check} from "./interfaces/IAdvancedChecker.sol"; +/// @notice Tracks validation status for pre, main, and post checks. +/// @dev Used to maintain check state in AdvancedPolicy. struct CheckStatus { + /// @dev Pre-check completion status. bool pre; + /// @dev Number of completed main checks. uint8 main; + /// @dev Post-check completion status. bool post; } /// @title AdvancedChecker. -/// @notice Abstract base contract which can be extended to implement a specific `AdvancedChecker`. -/// @dev The `AdvancedChecker` contract builds upon the `BaseChecker` by introducing additional validation phases. -/// It allows for pre-condition (`PRE`), main (`MAIN`), and post-condition (`POST`) checks, with the option to skip -/// pre and post checks based on constructor parameters. The `_check` method orchestrates the validation process -/// based on the specified check type. +/// @notice Multi-phase validation checker with pre, main, and post checks. +/// @dev Base contract for implementing complex validation logic with configurable check phases. abstract contract AdvancedChecker is IAdvancedChecker { - /// @notice Flag to determine if pre-condition checks should be skipped. + /// @notice Controls whether pre-condition checks are required. bool public immutable SKIP_PRE; - /// @notice Flag to determine if post-condition checks should be skipped. + /// @notice Controls whether post-condition checks are required. bool public immutable SKIP_POST; - /// @notice Flag to determine if main checks can be executed multiple times. + /// @notice Controls whether main check can be executed multiple times. bool public immutable ALLOW_MULTIPLE_MAIN; - /// @param _skipPre Indicates whether to skip pre-condition checks. - /// @param _skipPost Indicates whether to skip post-condition checks. - /// @param _allowMultipleMain Indicates whether the main check can be executed multiple times. + /// @notice Sets up checker configuration. + /// @param _skipPre Skip pre-condition validation. + /// @param _skipPost Skip post-condition validation. + /// @param _allowMultipleMain Allow multiple main validations. constructor(bool _skipPre, bool _skipPost, bool _allowMultipleMain) { SKIP_PRE = _skipPre; SKIP_POST = _skipPost; ALLOW_MULTIPLE_MAIN = _allowMultipleMain; } - /// @notice Public method to check the validity of the provided evidence for a given address and check type. - /// @param subject The address to be checked. - /// @param evidence The evidence associated with the check. - /// @param checkType The type of check to perform (PRE, MAIN, POST). + /// @notice Entry point for validation checks. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @param checkType Type of check (PRE, MAIN, POST). + /// @return checked Validation result. function check( address subject, bytes memory evidence, @@ -46,37 +50,46 @@ abstract contract AdvancedChecker is IAdvancedChecker { return _check(subject, evidence, checkType); } - /// @notice Internal method to orchestrate the validation process based on the specified check type. - /// @param subject The address to be checked. - /// @param evidence The evidence associated with the check. - /// @param checkType The type of check to perform (PRE, MAIN, POST). + /// @notice Core validation logic router. + /// @dev Directs to appropriate check based on type and configuration. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @param checkType Check type to perform. + /// @return checked Validation result. + /// @custom:throws PreCheckSkipped If PRE check attempted when skipped. + /// @custom:throws PostCheckSkipped If POST check attempted when skipped. function _check(address subject, bytes memory evidence, Check checkType) internal view returns (bool checked) { - if (SKIP_PRE && checkType == Check.PRE) revert PreCheckSkipped(); - if (SKIP_POST && checkType == Check.POST) revert PostCheckSkipped(); + // Validate skip conditions first. + if (checkType == Check.PRE && SKIP_PRE) revert PreCheckSkipped(); + if (checkType == Check.POST && SKIP_POST) revert PostCheckSkipped(); - if (!SKIP_PRE && checkType == Check.PRE) { - return _checkPre(subject, evidence); - } - - if (!SKIP_POST && checkType == Check.POST) { - return _checkPost(subject, evidence); - } - - return _checkMain(subject, evidence); + // Route to appropriate check. + return + checkType == Check.PRE + ? _checkPre(subject, evidence) + : checkType == Check.POST + ? _checkPost(subject, evidence) + : _checkMain(subject, evidence); } - /// @notice Internal method for performing pre-condition checks. - /// @param subject The address to be checked. - /// @param evidence The evidence associated with the check. + /// @notice Pre-condition validation implementation. + /// @dev Override to implement pre-check logic. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @return checked Validation result. function _checkPre(address subject, bytes memory evidence) internal view virtual returns (bool checked) {} - /// @notice Internal method for performing main checks. - /// @param subject The address to be checked. - /// @param evidence The evidence associated with the check. + /// @notice Main validation implementation. + /// @dev Override to implement main check logic. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @return checked Validation result. function _checkMain(address subject, bytes memory evidence) internal view virtual returns (bool checked) {} - /// @notice Internal method for performing post-condition checks. - /// @param subject The address to be checked. - /// @param evidence The evidence associated with the check. + /// @notice Post-condition validation implementation. + /// @dev Override to implement post-check logic. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @return checked Validation result. function _checkPost(address subject, bytes memory evidence) internal view virtual returns (bool checked) {} } diff --git a/packages/contracts/contracts/src/AdvancedPolicy.sol b/packages/contracts/contracts/src/AdvancedPolicy.sol index d8bc928..9df5867 100644 --- a/packages/contracts/contracts/src/AdvancedPolicy.sol +++ b/packages/contracts/contracts/src/AdvancedPolicy.sol @@ -5,80 +5,84 @@ import {Policy} from "./Policy.sol"; import {IAdvancedPolicy, Check} from "./interfaces/IAdvancedPolicy.sol"; import {AdvancedChecker, CheckStatus} from "./AdvancedChecker.sol"; -/// @title AdvancedPolicy -/// @notice Abstract base contract which can be extended to implement a specific `AdvancedPolicy`. +/// @title AdvancedPolicy. +/// @notice Implements advanced policy checks with pre, main, and post validation stages. +/// @dev Extends Policy contract with multi-stage validation capabilities. abstract contract AdvancedPolicy is IAdvancedPolicy, Policy { - /// @dev Reference to the AdvancedChecker contract for validation. + /// @notice Reference to the validation checker contract. + /// @dev Immutable to ensure checker cannot be changed after deployment. AdvancedChecker public immutable ADVANCED_CHECKER; - /// @dev Tracks the check status of each address. + /// @notice Tracks validation status for each subject per target. + /// @dev Maps target => subject => CheckStatus. mapping(address => mapping(address => CheckStatus)) public enforced; - /// @notice Constructor to initialize the AdvancedChecker contract. - /// @param _advancedChecker The address of the AdvancedChecker contract. + /// @notice Initializes contract with an AdvancedChecker instance. + /// @param _advancedChecker Address of the AdvancedChecker contract. constructor(AdvancedChecker _advancedChecker) { ADVANCED_CHECKER = _advancedChecker; } - /// @notice Enforces the custom target logic. - /// @dev Calls the internal `_enforce` function to enforce the target logic. - /// @dev Must call the `check` to handle the logic of checking subject for specific target. - /// @param subject The address of those who have successfully enforced the check. - /// @param evidence Additional data required for the check (e.g., encoded token identifier). - /// @param checkType The type of the check to be enforced for the subject with the given data. + /// @notice Enforces policy check for a subject. + /// @dev Only callable by target contract. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @param checkType Type of check (PRE, MAIN, POST). function enforce(address subject, bytes calldata evidence, Check checkType) external override onlyTarget { _enforce(subject, evidence, checkType); } - /// @notice Internal function to enforce the target logic. - /// @param subject The address of those who have successfully enforced the check. - /// @param evidence Additional data required for the check (e.g., encoded token identifier). - /// @param checkType The type of the check to be enforced for the subject with the given data. + /// @notice Internal check enforcement logic. + /// @dev Handles different check types and their dependencies. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @param checkType Type of check to perform. + /// @custom:throws UnsuccessfulCheck If validation fails. + /// @custom:throws AlreadyEnforced If check was already completed. + /// @custom:throws PreCheckNotEnforced If PRE check is required but not done. + /// @custom:throws MainCheckNotEnforced If MAIN check is required but not done. + /// @custom:throws MainCheckAlreadyEnforced If multiple MAIN checks not allowed. function _enforce(address subject, bytes calldata evidence, Check checkType) internal { - bool checked = ADVANCED_CHECKER.check(subject, evidence, checkType); - - if (!checked) { + if (!ADVANCED_CHECKER.check(subject, evidence, checkType)) { revert UnsuccessfulCheck(); } + CheckStatus storage status = enforced[msg.sender][subject]; + + // Handle PRE check. if (checkType == Check.PRE) { - if (!ADVANCED_CHECKER.SKIP_POST() && enforced[msg.sender][subject].pre) { + if (!ADVANCED_CHECKER.SKIP_POST() && status.pre) { revert AlreadyEnforced(); - } else { - enforced[msg.sender][subject].pre = true; } - } else { - if (checkType == Check.POST) { - if (enforced[msg.sender][subject].post) { - revert AlreadyEnforced(); - } else { - if (!ADVANCED_CHECKER.SKIP_PRE() && !enforced[msg.sender][subject].pre) { - revert PreCheckNotEnforced(); - } else { - if (enforced[msg.sender][subject].main == 0) { - revert MainCheckNotEnforced(); - } else { - enforced[msg.sender][subject].post = true; - } - } - } - } else { - if ( - checkType == Check.MAIN && - !ADVANCED_CHECKER.ALLOW_MULTIPLE_MAIN() && - enforced[msg.sender][subject].main > 0 - ) { - revert MainCheckAlreadyEnforced(); - } else { - if (checkType == Check.MAIN && !ADVANCED_CHECKER.SKIP_PRE() && !enforced[msg.sender][subject].pre) { - revert PreCheckNotEnforced(); - } else { - enforced[msg.sender][subject].main += 1; - } - } + status.pre = true; + emit Enforced(subject, target, evidence, checkType); + return; + } + + // Handle POST check. + if (checkType == Check.POST) { + if (status.post) { + revert AlreadyEnforced(); } + if (!ADVANCED_CHECKER.SKIP_PRE() && !status.pre) { + revert PreCheckNotEnforced(); + } + if (status.main == 0) { + revert MainCheckNotEnforced(); + } + status.post = true; + emit Enforced(subject, target, evidence, checkType); + return; } + // Handle MAIN check. + if (!ADVANCED_CHECKER.ALLOW_MULTIPLE_MAIN() && status.main > 0) { + revert MainCheckAlreadyEnforced(); + } + if (!ADVANCED_CHECKER.SKIP_PRE() && !status.pre) { + revert PreCheckNotEnforced(); + } + status.main += 1; emit Enforced(subject, target, evidence, checkType); } } diff --git a/packages/contracts/contracts/src/BaseChecker.sol b/packages/contracts/contracts/src/BaseChecker.sol index 36268b5..b59440e 100644 --- a/packages/contracts/contracts/src/BaseChecker.sol +++ b/packages/contracts/contracts/src/BaseChecker.sol @@ -3,21 +3,24 @@ pragma solidity 0.8.27; import {IBaseChecker} from "./interfaces/IBaseChecker.sol"; -/// @title BaseChecker. -/// @notice Abstract base contract which can be extended to implement a specific `BaseChecker`. -/// @dev The `BaseChecker` contract provides a foundational structure for implementing specific checker logic. -/// It defines a method `check` that invokes a protected `_check` method, which must be implemented by derived -/// contracts. +/// @title BaseChecker +/// @notice Abstract base contract for implementing validation checks. +/// @dev Provides a standardized interface for implementing custom validation logic +/// through the internal _check method. abstract contract BaseChecker is IBaseChecker { - /// @notice Checks the validity of the provided evidence for a given address. - /// @param subject The address to be checked. - /// @param evidence The evidence associated with the check. + /// @notice Validates evidence for a given subject address. + /// @dev External view function that delegates to internal _check implementation. + /// @param subject Address to validate. + /// @param evidence Custom validation data. + /// @return checked Boolean indicating if the check passed. function check(address subject, bytes memory evidence) external view override returns (bool checked) { return _check(subject, evidence); } - /// @notice Internal method to perform the actual check logic. - /// @param subject The address to be checked. - /// @param evidence The evidence associated with the check. + /// @notice Internal validation logic implementation. + /// @dev Must be implemented by derived contracts. + /// @param subject Address to validate. + /// @param evidence Custom validation data. + /// @return checked Boolean indicating if the check passed. function _check(address subject, bytes memory evidence) internal view virtual returns (bool checked) {} } diff --git a/packages/contracts/contracts/src/BasePolicy.sol b/packages/contracts/contracts/src/BasePolicy.sol index ca7fae1..5903df7 100644 --- a/packages/contracts/contracts/src/BasePolicy.sol +++ b/packages/contracts/contracts/src/BasePolicy.sol @@ -6,31 +6,45 @@ import {Policy} from "./Policy.sol"; import {BaseChecker} from "./BaseChecker.sol"; /// @title BasePolicy -/// @notice Abstract base contract which can be extended to implement a specific `BasePolicy`. +/// @notice Abstract base contract for implementing specific policy checks. +/// @dev Inherits from Policy and implements IBasePolicy interface. +/// +/// Provides core functionality for enforcing policy checks through a BaseChecker +/// contract. Each specific policy implementation should extend this contract +/// and implement its custom checking logic. abstract contract BasePolicy is Policy, IBasePolicy { - /// @dev Reference to the BaseChecker contract for validation. + /// @notice Reference to the BaseChecker contract used for validation. + /// @dev Immutable to ensure checker cannot be changed after deployment. BaseChecker public immutable BASE_CHECKER; - /// @dev Tracks whether the check has been enforced for a subject. + /// @notice Tracks enforcement status for each subject per target. + /// @dev Maps target => subject => enforcement status. mapping(address => mapping(address => bool)) public enforced; - /// @notice Constructor to initialize the BaseChecker contract. - /// @param _baseChecker The address of the BaseChecker contract. + /// @notice Initializes the contract with a BaseChecker instance. + /// @param _baseChecker Address of the BaseChecker contract. + /// @dev The BaseChecker address cannot be changed after deployment. constructor(BaseChecker _baseChecker) { BASE_CHECKER = _baseChecker; } - /// @notice Enforces the custom target enforcing logic. - /// @dev Must call the `check` to handle the logic of checking subject for specific target. - /// @param subject The address of those who have successfully enforced the check. - /// @param evidence Additional data required for the check (e.g., encoded token identifier). + /// @notice External function to enforce policy checks. + /// @dev Only callable by the target contract. + /// @param subject Address to enforce the check on. + /// @param evidence Additional data required for verification. + /// @custom:throws AlreadyEnforced if check was previously enforced. + /// @custom:throws UnsuccessfulCheck if the check fails. + /// @custom:emits Enforced when check succeeds. function enforce(address subject, bytes calldata evidence) external override onlyTarget { _enforce(subject, evidence); } - /// @notice Enforces the custom target enforcing logic. - /// @param subject The address of those who have successfully enforced the check. - /// @param evidence Additional data required for the check (e.g., encoded token identifier). + /// @notice Internal implementation of enforcement logic. + /// @dev Performs the actual check using BASE_CHECKER. + /// @param subject Address to enforce the check on. + /// @param evidence Additional data required for verification. + /// @custom:throws AlreadyEnforced if already enforced for this subject. + /// @custom:throws UnsuccessfulCheck if BASE_CHECKER.check returns false. function _enforce(address subject, bytes calldata evidence) internal { bool checked = BASE_CHECKER.check(subject, evidence); diff --git a/packages/contracts/contracts/src/Policy.sol b/packages/contracts/contracts/src/Policy.sol index 410671a..451964f 100644 --- a/packages/contracts/contracts/src/Policy.sol +++ b/packages/contracts/contracts/src/Policy.sol @@ -4,24 +4,33 @@ pragma solidity 0.8.27; import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; import {IPolicy} from "./interfaces/IPolicy.sol"; -/// @title Policy abstract contract. -/// @dev This contract implements the IPolicy interface and manages the target address. +/// @title Policy +/// @notice Implements a base policy contract that protects access to a target contract +/// @dev Inherits from OpenZeppelin's Ownable and implements IPolicy interface +/// +/// This contract serves as a base for implementing specific policy checks that must be +/// satisfied before interacting with a protected target contract. It provides core +/// functionality for managing the protected target address and access control. abstract contract Policy is IPolicy, Ownable(msg.sender) { - /// @notice The Policy-protected contract address. - /// @dev The target can be any contract address that requires a prior check to enable logic. + /// @notice The policy-protected contract address. + /// @dev This address can only be set once by the owner. /// For example, the target is a Semaphore group that requires the subject - /// to meet certain criteria before joining. + /// to meet certain criteria in order to join the group. address internal target; - /// @notice Modifier that restricts access to the target address. + /// @notice Restricts function access to only the target contract. + /// @dev Throws TargetOnly error if called by any other address. modifier onlyTarget() { if (msg.sender != target) revert TargetOnly(); _; } - /// @notice Sets the target address. - /// @dev Only the owner can set the destination `target` address. - /// @param _target The address of the contract to be set as the target. + /// @notice Sets the target contract address. + /// @dev Can only be called once by the owner. + /// @param _target Address of the contract to be protected by this policy. + /// @custom:throws ZeroAddress if _target is the zero address. + /// @custom:throws TargetAlreadySet if target has already been set. + /// @custom:emits TargetSet when target is successfully set. function setTarget(address _target) public virtual onlyOwner { if (_target == address(0)) revert ZeroAddress(); if (target != address(0)) revert TargetAlreadySet(); @@ -31,8 +40,9 @@ abstract contract Policy is IPolicy, Ownable(msg.sender) { emit TargetSet(_target); } - /// @notice Retrieves the current target address. - /// @return The address of the current target. + /// @notice Retrieves the current target contract address. + /// @return address The address of the policy-protected contract. + /// @dev Returns zero address if target hasn't been set yet. function getTarget() public view returns (address) { return target; } diff --git a/packages/contracts/contracts/src/interfaces/IAdvancedChecker.sol b/packages/contracts/contracts/src/interfaces/IAdvancedChecker.sol index 60a620e..90a9d17 100644 --- a/packages/contracts/contracts/src/interfaces/IAdvancedChecker.sol +++ b/packages/contracts/contracts/src/interfaces/IAdvancedChecker.sol @@ -1,11 +1,11 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.27; -/// @notice This enum defines the types of checks that can be performed in the AdvancedChecker system. -/// @dev The `Check` enum represents the different phases of validation in the AdvancedChecker system. -/// - `PRE`: Represents the pre-condition check that must be satisfied before the `MAIN` check can occur. -/// - `MAIN`: The primary check that is executed, which can be validated multiple times. -/// - `POST`: Represents the post-condition check that can be validated after the `MAIN` check has been completed. +/// @title Check. +/// @notice Defines validation phases in the AdvancedChecker system. +/// @custom:values PRE - Pre-condition validation. +/// MAIN - Primary validation. +/// POST - Post-condition validation. enum Check { PRE, MAIN, @@ -13,17 +13,19 @@ enum Check { } /// @title IAdvancedChecker. -/// @notice AdvancedChecker contract interface. +/// @notice Defines multi-phase validation system interface. +/// @dev Implement this for custom validation logic with pre/main/post checks. interface IAdvancedChecker { - /// @notice Error thrown when the PRE check is skipped. + /// @notice Thrown when pre-check validation attempted while disabled. error PreCheckSkipped(); - /// @notice Error thrown when the POST check is skipped. + + /// @notice Thrown when post-check validation attempted while disabled. error PostCheckSkipped(); - /// @dev Defines the custom `target` protection logic. - /// @param subject The address of the entity attempting to interact with the `target`. - /// @param evidence Additional data that may be required for the check. - /// @param checkType The type of check to be enforced (e.g., PRE, MAIN, POST). - /// @return checked A boolean that resolves to true when the subject satisfies the checks; otherwise false. + /// @notice Validates subject against specified check type. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @param checkType Check phase to execute. + /// @return checked True if validation passes. function check(address subject, bytes calldata evidence, Check checkType) external view returns (bool checked); } diff --git a/packages/contracts/contracts/src/interfaces/IAdvancedPolicy.sol b/packages/contracts/contracts/src/interfaces/IAdvancedPolicy.sol index 36f4b80..c6cfc71 100644 --- a/packages/contracts/contracts/src/interfaces/IAdvancedPolicy.sol +++ b/packages/contracts/contracts/src/interfaces/IAdvancedPolicy.sol @@ -4,31 +4,29 @@ pragma solidity 0.8.27; import {IPolicy} from "./IPolicy.sol"; import {Check} from "./IAdvancedChecker.sol"; -/// @title IAdvancedPolicy -/// @notice IAdvancedPolicy contract interface that extends the IPolicy interface. +/// @title IAdvancedPolicy. +/// @notice Extends IPolicy with multi-phase validation capabilities. interface IAdvancedPolicy is IPolicy { - /// @notice Error thrown when the MAIN check cannot be executed more than once. + /// @notice Thrown when multiple main checks not allowed. error MainCheckAlreadyEnforced(); - /// @notice Error thrown when the PRE check has not been enforced yet. + /// @notice Thrown when main check attempted before pre-check. error PreCheckNotEnforced(); - /// @notice Error thrown when the MAIN check has not been enforced yet. + /// @notice Thrown when post check attempted before main check. error MainCheckNotEnforced(); - /// @notice Event emitted when someone enforces the `target` check. - /// @param subject The address of those who have successfully enforced the check. - /// @param target The address of the policy-protected contract address. - /// @param evidence Additional data required for the check (e.g., encoded token identifier). - /// @param checkType The type of check that was performed (e.g., PRE, MAIN, POST). + /// @notice Emitted when validation check succeeds. + /// @param subject Address that passed validation. + /// @param target Protected contract address. + /// @param evidence Validation data. + /// @param checkType Type of check performed. event Enforced(address indexed subject, address indexed target, bytes evidence, Check checkType); - /// @notice Enforces the custom target enforcing logic. - /// @dev Must call the right `check` method based on the `checkType` to handle the logic of checking - /// subject for specific target. - /// @dev Must call the `check` to handle the logic of checking subject for specific target. - /// @param subject The address of those who have successfully enforced the check. - /// @param evidence Additional data required for the check (e.g., encoded token identifier). - /// @param checkType The type of the check to be enforced for the subject with the given data. + /// @notice Enforces validation check on subject. + /// @dev Delegates to appropriate check method based on checkType. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @param checkType Check phase to execute. function enforce(address subject, bytes calldata evidence, Check checkType) external; } diff --git a/packages/contracts/contracts/src/interfaces/IBaseChecker.sol b/packages/contracts/contracts/src/interfaces/IBaseChecker.sol index e7fdfee..ad83c82 100644 --- a/packages/contracts/contracts/src/interfaces/IBaseChecker.sol +++ b/packages/contracts/contracts/src/interfaces/IBaseChecker.sol @@ -1,12 +1,12 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.27; -/// @title IBaseChecker -/// @notice BaseChecker contract interface that defines the basic check functionality. +/// @title IBaseChecker. +/// @notice Defines base validation functionality. interface IBaseChecker { - /// @dev Defines the custom `target` protection logic. - /// @param subject The address of the entity attempting to the `target`. - /// @param evidence Additional data required for the check (e.g., encoded token identifier). - /// @return checked A boolean that resolves to true when the subject satisfies the checks; otherwise false. + /// @notice Validates subject against evidence. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @return checked True if validation passes. function check(address subject, bytes calldata evidence) external view returns (bool checked); } diff --git a/packages/contracts/contracts/src/interfaces/IBasePolicy.sol b/packages/contracts/contracts/src/interfaces/IBasePolicy.sol index e45d880..fe11da4 100644 --- a/packages/contracts/contracts/src/interfaces/IBasePolicy.sol +++ b/packages/contracts/contracts/src/interfaces/IBasePolicy.sol @@ -3,18 +3,17 @@ pragma solidity 0.8.27; import {IPolicy} from "./IPolicy.sol"; -/// @title IBasePolicy -/// @notice BasePolicy contract interface that extends the IPolicy interface. +/// @title IBasePolicy. +/// @notice Extends IPolicy with basic validation capabilities. interface IBasePolicy is IPolicy { - /// @notice Event emitted when someone enforcing the `target` check. - /// @param subject The address of those who have successfully enforced the check. - /// @param target The address of the policy-protected contract address. - /// @param evidence Additional data required for the check (e.g., encoded token identifier). + /// @notice Emitted when validation succeeds. + /// @param subject Address that passed validation. + /// @param target Protected contract address. + /// @param evidence Validation data. event Enforced(address indexed subject, address indexed target, bytes evidence); - /// @notice Enforces the custom target enforcing logic. - /// @dev Must call the `check` to handle the logic of checking subject for specific target. - /// @param subject The address of those who have successfully enforced the check. - /// @param evidence Additional data required for the check (e.g., encoded token identifier). + /// @notice Enforces validation check on subject. + /// @param subject Address to validate. + /// @param evidence Validation data. function enforce(address subject, bytes calldata evidence) external; } diff --git a/packages/contracts/contracts/src/interfaces/IPolicy.sol b/packages/contracts/contracts/src/interfaces/IPolicy.sol index 18bbb5d..b1afe1f 100644 --- a/packages/contracts/contracts/src/interfaces/IPolicy.sol +++ b/packages/contracts/contracts/src/interfaces/IPolicy.sol @@ -1,37 +1,26 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.27; -/// @title IPolicy -/// @notice Policy contract interface that defines the basic functionalities for `target` protected contract management. +/// @title IPolicy. +/// @notice Core policy interface for protected contract management. interface IPolicy { - /// @notice Event emitted when the `target` address is set. - /// @param target The address of the contract set as the `target`. + /// @notice Emitted when target contract is set. event TargetSet(address indexed target); - /// @notice Error thrown when an address equal to zero is given. + /// @notice Core error conditions. error ZeroAddress(); - - /// @notice Error thrown when a subject do not satisfy the checks. error UnsuccessfulCheck(); - - /// @notice Error thrown when the `target` address is not set. error TargetNotSet(); - - /// @notice Error thrown when the callee is not the `target` contract. error TargetOnly(); - - /// @notice Error thrown when the `target` address has been already set. error TargetAlreadySet(); - - /// @notice Error thrown when the subject has already enforced the `target`. error AlreadyEnforced(); - /// @notice Gets the trait of the Policy contract. - /// @return The specific trait of the Policy contract (e.g., SemaphorePolicy has trait Semaphore). + /// @notice Returns policy trait identifier. + /// @return Policy trait string (e.g., "Semaphore"). function trait() external pure returns (string memory); - /// @notice Sets the target address. - /// @dev Only the owner can set the destination `target` address. - /// @param _target The address of the contract to be set as the target. + /// @notice Sets protected contract address. + /// @dev Owner-only, one-time setting. + /// @param _target Protected contract address. function setTarget(address _target) external; } diff --git a/packages/contracts/contracts/src/test/Advanced.t.sol b/packages/contracts/contracts/src/test/Advanced.t.sol index b9624fc..f35e2fd 100644 --- a/packages/contracts/contracts/src/test/Advanced.t.sol +++ b/packages/contracts/contracts/src/test/Advanced.t.sol @@ -34,7 +34,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_pre_RevertWhen_ERC721NonexistentToken() public { + function test_checkPre_whenTokenDoesNotExist_reverts() public { vm.startPrank(target); vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, uint256(0))); @@ -43,7 +43,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_pre_return_False() public { + function test_checkPre_whenCallerNotOwner_returnsFalse() public { vm.startPrank(target); nft.mint(subject); @@ -53,7 +53,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_pre() public { + function test_checkPre_whenValid_succeeds() public { vm.startPrank(target); nft.mint(subject); @@ -63,7 +63,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_main_return_False() public { + function test_checkMain_whenCallerHasNoTokens_returnsFalse() public { vm.startPrank(target); nft.mint(subject); @@ -73,7 +73,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_main() public { + function test_checkMain_whenCallerHasTokens_succeeds() public { vm.startPrank(target); nft.mint(subject); @@ -83,7 +83,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_post_RevertWhen_ERC721NonexistentToken() public { + function test_checkPost_whenTokenDoesNotExist_reverts() public { vm.startPrank(target); vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, uint256(1))); @@ -92,7 +92,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_post_return_False() public { + function test_checkPost_whenCallerNotOwner_returnsFalse() public { vm.startPrank(target); nft.mint(subject); @@ -102,7 +102,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_post() public { + function test_checkPost_whenValid_succeeds() public { vm.startPrank(target); nft.mint(subject); @@ -112,7 +112,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_pre_internal_RevertWhen_ERC721NonexistentToken() public { + function test_checkerPre_whenTokenDoesNotExist_reverts() public { vm.startPrank(target); vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, uint256(0))); @@ -121,7 +121,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_pre_internal_return_False() public { + function test_checkerPre_whenCallerNotOwner_returnsFalse() public { vm.startPrank(target); nft.mint(subject); @@ -131,7 +131,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_pre_internal() public { + function test_checkerPre_whenValid_succeeds() public { vm.startPrank(target); nft.mint(subject); @@ -141,7 +141,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_main_internal_return_False() public { + function test_checkerMain_whenCallerHasNoTokens_returnsFalse() public { vm.startPrank(target); nft.mint(subject); @@ -151,7 +151,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_main_internal() public { + function test_checkerMain_whenCallerHasTokens_succeeds() public { vm.startPrank(target); nft.mint(subject); @@ -161,7 +161,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_post_internal_RevertWhen_ERC721NonexistentToken() public { + function test_checkerPost_whenTokenDoesNotExist_reverts() public { vm.startPrank(target); vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, uint256(1))); @@ -170,7 +170,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_post_internal_return_False() public { + function test_checkerPost_whenCallerNotOwner_returnsFalse() public { vm.startPrank(target); nft.mint(subject); @@ -180,7 +180,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_check_post_internal() public { + function test_checkerPost_whenValid_succeeds() public { vm.startPrank(target); nft.mint(subject); @@ -190,7 +190,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_checkPre_internal_RevertWhen_ERC721NonexistentToken() public { + function test_internalPre_whenTokenDoesNotExist_reverts() public { vm.startPrank(target); vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, uint256(1))); @@ -199,7 +199,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_checkPre_internal_return_False() public { + function test_internalPre_whenCallerNotOwner_returnsFalse() public { vm.startPrank(target); nft.mint(subject); @@ -209,7 +209,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_checkPre() public { + function test_internalPre_whenValid_succeeds() public { vm.startPrank(target); nft.mint(subject); @@ -219,7 +219,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_checkMain_internal_return_False() public { + function test_internalMain_whenCallerHasNoTokens_returnsFalse() public { vm.startPrank(target); nft.mint(subject); @@ -229,7 +229,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_checkMain() public { + function test_internalMain_whenCallerHasTokens_succeeds() public { vm.startPrank(target); nft.mint(subject); @@ -239,7 +239,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_checkPost_internal_RevertWhen_ERC721NonexistentToken() public { + function test_internalPost_whenTokenDoesNotExist_reverts() public { vm.startPrank(target); vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, uint256(1))); @@ -248,7 +248,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_checkPost_internal_return_False() public { + function test_internalPost_whenCallerNotOwner_returnsFalse() public { vm.startPrank(target); nft.mint(subject); @@ -258,7 +258,7 @@ contract AdvancedChecker is Test { vm.stopPrank(); } - function test_checkPost() public { + function test_internalPost_whenValid_succeeds() public { vm.startPrank(target); nft.mint(subject); @@ -297,11 +297,11 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_trait() public view { + function test_trait_returnsCorrectValue() public view { assertEq(policy.trait(), "AdvancedERC721"); } - function test_setTarget_RevertWhen_OwnableUnauthorizedAccount() public { + function test_setTarget_whenCallerNotOwner_reverts() public { vm.startPrank(notOwner); vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notOwner)); @@ -310,7 +310,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_setTarget_RevertWhen_ZeroAddress() public { + function test_setTarget_whenZeroAddress_reverts() public { vm.startPrank(deployer); vm.expectRevert(abi.encodeWithSelector(IPolicy.ZeroAddress.selector)); @@ -319,7 +319,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_setTarget() public { + function test_setTarget_whenValid_succeeds() public { vm.startPrank(deployer); vm.expectEmit(true, true, true, true); @@ -330,7 +330,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_setTarget_RevertWhen_TargetAlreadySet() public { + function test_setTarget_whenAlreadySet_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -341,7 +341,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre_RevertWhen_TargetOnly() public { + function test_enforcePre_whenCallerNotTarget_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -356,7 +356,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre_RevertWhen_ERC721NonexistentToken() public { + function test_enforcePre_whenTokenDoesNotExist_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -371,7 +371,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre_RevertWhen_PreCheckSkipped() public { + function test_enforcePre_whenChecksSkipped_reverts() public { vm.startPrank(deployer); policySkipped.setTarget(target); @@ -386,7 +386,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre_RevertWhen_UnsuccessfulCheck() public { + function test_enforcePre_whenCheckFails_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -402,7 +402,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre() public { + function test_enforcePre_whenValid_succeeds() public { vm.startPrank(deployer); policy.setTarget(target); @@ -420,7 +420,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre_RevertWhen_AlreadyEnforced() public { + function test_enforcePre_whenAlreadyEnforced_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -438,7 +438,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main_RevertWhen_TargetOnly() public { + function test_enforceMain_whenCallerNotTarget_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -453,7 +453,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main_RevertWhen_UnsuccessfulCheck() public { + function test_enforceMain_whenCheckFails_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -468,7 +468,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main_RevertWhen_PreCheckNotEnforced() public { + function test_enforceMain_whenPreCheckMissing_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -484,7 +484,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main() public { + function test_enforceMain_whenValid_succeeds() public { vm.startPrank(deployer); policy.setTarget(target); @@ -504,7 +504,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main_twice() public { + function test_enforceMain_whenMultipleValid_succeeds() public { vm.startPrank(deployer); policy.setTarget(target); @@ -529,7 +529,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main_RevertWhen_MainCheckAlreadyEnforced() public { + function test_enforceMain_whenMultipleNotAllowed_reverts() public { vm.startPrank(deployer); policySkipped.setTarget(target); @@ -547,7 +547,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_RevertWhen_PreCheckNotEnforced() public { + function test_enforcePost_whenPreCheckMissing_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -564,7 +564,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_RevertWhen_TargetOnly() public { + function test_enforcePost_whenCallerNotTarget_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -579,7 +579,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_RevertWhen_ERC721NonexistentToken() public { + function test_enforcePost_whenTokenDoesNotExist_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -597,7 +597,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_RevertWhen_PreCheckSkipped() public { + function test_enforcePost_whenChecksSkipped_reverts() public { vm.startPrank(deployer); policySkipped.setTarget(target); @@ -615,7 +615,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_RevertWhen_UnsuccessfulCheck() public { + function test_enforcePost_whenCheckFails_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -634,7 +634,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post() public { + function test_enforcePost_whenValid_succeeds() public { vm.startPrank(deployer); policy.setTarget(target); @@ -655,7 +655,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_RevertWhen_AlreadyEnforced() public { + function test_enforcePost_whenAlreadyEnforced_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -675,7 +675,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre_internal_RevertWhen_TargetOnly() public { + function test_enforcePreInternal_whenCallerNotTarget_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -690,7 +690,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre_internal_RevertWhen_ERC721NonexistentToken() public { + function test_enforcePreInternal_whenTokenDoesNotExist_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -705,7 +705,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre_internal_RevertWhen_PreCheckSkipped() public { + function test_enforcePreInternal_whenChecksSkipped_reverts() public { vm.startPrank(deployer); policyHarnessSkipped.setTarget(target); @@ -720,7 +720,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre_internal_RevertWhen_UnsuccessfulCheck() public { + function test_enforcePreInternal_whenCheckFails_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -736,7 +736,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre_internal() public { + function test_enforcePreInternal_whenValid_succeeds() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -754,7 +754,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_pre_internal_RevertWhen_AlreadyEnforced() public { + function test_enforcePreInternal_whenAlreadyEnforced_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -772,7 +772,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main_internal_RevertWhen_TargetOnly() public { + function test_enforceMainInternal_whenCallerNotTarget_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -787,7 +787,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main_internal_RevertWhen_UnsuccessfulCheck() public { + function test_enforceMainInternal_whenCheckFails_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -802,7 +802,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main_internal_RevertWhen_PreCheckNotEnforced() public { + function test_enforceMainInternal_whenPreCheckMissing_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -818,7 +818,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main_internal() public { + function test_enforceMainInternal_whenValid_succeeds() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -838,7 +838,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main_internal_twice() public { + function test_enforceMainInternal_whenMultipleValid_succeeds() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -863,7 +863,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_main_internal_RevertWhen_MainCheckAlreadyEnforced() public { + function test_enforceMainInternal_whenMultipleNotAllowed_reverts() public { vm.startPrank(deployer); policyHarnessSkipped.setTarget(target); @@ -881,7 +881,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_internal_RevertWhen_PreCheckNotEnforced() public { + function test_enforcePostInternal_whenPreCheckMissing_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -898,7 +898,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_internal_RevertWhen_TargetOnly() public { + function test_enforcePostInternal_whenCallerNotTarget_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -913,7 +913,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_internal_RevertWhen_ERC721NonexistentToken() public { + function test_enforcePostInternal_whenTokenDoesNotExist_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -931,7 +931,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_internal_RevertWhen_PreCheckSkipped() public { + function test_enforcePostInternal_whenChecksSkipped_reverts() public { vm.startPrank(deployer); policyHarnessSkipped.setTarget(target); @@ -949,7 +949,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_internal_RevertWhen_UnsuccessfulCheck() public { + function test_enforcePostInternal_whenCheckFails_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -968,7 +968,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_internal() public { + function test_enforcePostInternal_whenValid_succeeds() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -989,7 +989,7 @@ contract AdvancedPolicy is Test { vm.stopPrank(); } - function test_enforce_post_internal_RevertWhen_AlreadyEnforced() public { + function test_enforcePostInternal_whenAlreadyEnforced_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -1031,7 +1031,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_register_RevertWhen_TargetOnly() public { + function test_register_whenCallerNotTarget_reverts() public { vm.startPrank(deployer); policy.setTarget(deployer); @@ -1047,7 +1047,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_register_RevertWhen_ERC721NonexistentToken() public { + function test_register_whenTokenDoesNotExist_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1063,7 +1063,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_register_RevertWhen_UnsuccessfulCheck() public { + function test_register_whenCheckFails_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1079,7 +1079,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_register() public { + function test_register_whenValid_succeeds() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1097,7 +1097,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_register_RevertWhen_AlreadyEnforced() public { + function test_register_whenAlreadyRegistered_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1115,7 +1115,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_vote_RevertWhen_NotRegistered() public { + function test_vote_whenNotRegistered_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1131,7 +1131,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_vote_RevertWhen_InvalidOption() public { + function test_vote_whenInvalidOption_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1148,7 +1148,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_vote() public { + function test_vote_whenValid_succeeds() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1167,7 +1167,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_vote_twice() public { + function test_vote_whenMultipleValid_succeeds() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1187,7 +1187,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_reward_RevertWhen_ERC721NonexistentToken() public { + function test_reward_whenTokenDoesNotExist_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1206,7 +1206,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_reward_RevertWhen_UnsuccessfulCheck() public { + function test_reward_whenCheckFails_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1231,7 +1231,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_reward_RevertWhen_NotRegistered() public { + function test_reward_whenNotRegistered_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1247,7 +1247,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_reward_RevertWhen_NotVoted() public { + function test_reward_whenNotVoted_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1264,7 +1264,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_reward() public { + function test_reward_whenValid_succeeds() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -1285,7 +1285,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_reward_RevertWhen_AlreadyClaimed() public { + function test_reward_whenAlreadyClaimed_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); diff --git a/packages/contracts/contracts/src/test/Base.t.sol b/packages/contracts/contracts/src/test/Base.t.sol index 082eb64..2701b3d 100644 --- a/packages/contracts/contracts/src/test/Base.t.sol +++ b/packages/contracts/contracts/src/test/Base.t.sol @@ -33,7 +33,7 @@ contract BaseChecker is Test { vm.stopPrank(); } - function test_check_internal_RevertWhen_ERC721NonexistentToken() public { + function test_checker_whenTokenDoesNotExist_reverts() public { vm.startPrank(target); vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, uint256(0))); @@ -42,7 +42,7 @@ contract BaseChecker is Test { vm.stopPrank(); } - function test_check_internal_return_False() public { + function test_checker_whenCallerNotOwner_returnsFalse() public { vm.startPrank(target); nft.mint(subject); @@ -52,7 +52,7 @@ contract BaseChecker is Test { vm.stopPrank(); } - function test_check_internal() public { + function test_checker_whenCallerIsOwner_succeeds() public { vm.startPrank(target); nft.mint(subject); @@ -62,7 +62,7 @@ contract BaseChecker is Test { vm.stopPrank(); } - function test_check_RevertWhen_ERC721NonexistentToken() public { + function test_checkerExternal_whenTokenDoesNotExist_reverts() public { vm.startPrank(target); vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, uint256(0))); @@ -71,7 +71,7 @@ contract BaseChecker is Test { vm.stopPrank(); } - function test_check_return_False() public { + function test_checkerExternal_whenCallerNotOwner_returnsFalse() public { vm.startPrank(target); nft.mint(subject); @@ -81,7 +81,7 @@ contract BaseChecker is Test { vm.stopPrank(); } - function test_check() public { + function test_checkerExternal_whenCallerIsOwner_succeeds() public { vm.startPrank(target); nft.mint(subject); @@ -114,11 +114,11 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_trait() public view { + function test_trait_returnsCorrectValue() public view { assertEq(policy.trait(), "BaseERC721"); } - function test_getTarget() public { + function test_getTarget_returnsExpectedAddress() public { vm.startPrank(deployer); policy.setTarget(target); @@ -128,7 +128,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_setTarget_RevertWhen_OwnableUnauthorizedAccount() public { + function test_setTarget_whenCallerNotOwner_reverts() public { vm.startPrank(notOwner); vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notOwner)); @@ -137,7 +137,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_setTarget_RevertWhen_ZeroAddress() public { + function test_setTarget_whenZeroAddress_reverts() public { vm.startPrank(deployer); vm.expectRevert(abi.encodeWithSelector(IPolicy.ZeroAddress.selector)); @@ -146,7 +146,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_setTarget() public { + function test_setTarget_whenValidAddress_succeeds() public { vm.startPrank(deployer); vm.expectEmit(true, true, true, true); @@ -157,7 +157,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_setTarget_RevertWhen_TargetAlreadySet() public { + function test_setTarget_whenAlreadySet_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -168,7 +168,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_enforce_RevertWhen_TargetOnly() public { + function test_enforce_whenCallerNotTarget_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -183,7 +183,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_enforce_RevertWhen_ERC721NonexistentToken() public { + function test_enforce_whenTokenDoesNotExist_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -198,7 +198,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_enforce_RevertWhen_UnsuccessfulCheck() public { + function test_enforce_whenCheckFails_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -214,7 +214,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_enforce() public { + function test_enforce_whenValid_succeeds() public { vm.startPrank(deployer); policy.setTarget(target); @@ -232,7 +232,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_enforce_RevertWhen_AlreadyEnforced() public { + function test_enforce_whenAlreadyEnforced_reverts() public { vm.startPrank(deployer); policy.setTarget(target); @@ -250,7 +250,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_enforce_internal_RevertWhen_TargetOnly() public { + function test_enforceInternal_whenCallerNotTarget_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -265,7 +265,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_enforce_internal_RevertWhen_ERC721NonexistentToken() public { + function test_enforceInternal_whenTokenDoesNotExist_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -280,7 +280,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_enforce_internal_RevertWhen_UnsuccessfulCheck() public { + function test_enforceInternal_whenCheckFails_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -296,7 +296,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_enforce_internal() public { + function test_enforceInternal_whenValid_succeeds() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -314,7 +314,7 @@ contract BasePolicy is Test { vm.stopPrank(); } - function test_enforce_internal_RevertWhen_AlreadyEnforced() public { + function test_enforceInternal_whenAlreadyEnforced_reverts() public { vm.startPrank(deployer); policyHarness.setTarget(target); @@ -354,7 +354,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_register_RevertWhen_TargetOnly() public { + function test_register_whenCallerNotTarget_reverts() public { vm.startPrank(deployer); policy.setTarget(deployer); @@ -370,7 +370,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_register_RevertWhen_ERC721NonexistentToken() public { + function test_register_whenTokenDoesNotExist_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -386,7 +386,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_register_RevertWhen_UnsuccessfulCheck() public { + function test_register_whenCheckFails_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -402,7 +402,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_register() public { + function test_register_whenValid_succeeds() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -420,7 +420,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_register_RevertWhen_AlreadyEnforced() public { + function test_register_whenAlreadyRegistered_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -438,7 +438,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_vote_RevertWhen_NotRegistered() public { + function test_vote_whenNotRegistered_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -454,7 +454,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_vote_RevertWhen_InvalidOption() public { + function test_vote_whenInvalidOption_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -471,7 +471,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_vote() public { + function test_vote_whenValid_succeeds() public { vm.startPrank(deployer); policy.setTarget(address(voting)); @@ -490,7 +490,7 @@ contract Voting is Test { vm.stopPrank(); } - function test_vote_RevertWhen_AlreadyVoted() public { + function test_vote_whenAlreadyVoted_reverts() public { vm.startPrank(deployer); policy.setTarget(address(voting)); diff --git a/packages/contracts/contracts/src/test/advanced/AdvancedERC721Checker.sol b/packages/contracts/contracts/src/test/advanced/AdvancedERC721Checker.sol index 232e8e1..a2b6ad0 100644 --- a/packages/contracts/contracts/src/test/advanced/AdvancedERC721Checker.sol +++ b/packages/contracts/contracts/src/test/advanced/AdvancedERC721Checker.sol @@ -4,23 +4,24 @@ pragma solidity 0.8.27; import {AdvancedChecker} from "../../AdvancedChecker.sol"; import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; -/** - * @title AdvancedERC721Checker - * @notice Implements advanced checks for ERC721 token requirements. - * @dev Extends AdvancedChecker to provide three-phase validation: - * - Pre-check: Basic token ownership verification. - * - Main check: Token balance threshold validation. - * - Post-check: Special token ID range verification. - */ +/// @title AdvancedERC721Checker. +/// @notice Multi-phase ERC721 token validation. +/// @dev Implements pre, main, and post checks for NFTs. contract AdvancedERC721Checker is AdvancedChecker { + /// @notice Contract references and thresholds. IERC721 public immutable NFT; - /// @notice Minimum token balance required for main check. uint256 public immutable MIN_BALANCE; - /// @notice Minimum token ID allowed for post-check validation. uint256 public immutable MIN_TOKEN_ID; - /// @notice Maximum token ID allowed for post-check validation. uint256 public immutable MAX_TOKEN_ID; + /// @notice Initializes checker with validation parameters. + /// @param _nft ERC721 contract address. + /// @param _minBalance Required token balance. + /// @param _minTokenId Minimum valid token ID. + /// @param _maxTokenId Maximum valid token ID. + /// @param _skipPre Skip pre-check flag. + /// @param _skipPost Skip post-check flag. + /// @param _allowMultipleMain Allow multiple main checks flag. constructor( IERC721 _nft, uint256 _minBalance, @@ -36,43 +37,31 @@ contract AdvancedERC721Checker is AdvancedChecker { MAX_TOKEN_ID = _maxTokenId; } - /** - * @notice Pre-check verifies basic token ownership. - * @dev Validates if the subject owns the specific tokenId provided in evidence. - * @param subject Address to check ownership for. - * @param evidence Encoded uint256 tokenId. - * @return True if subject owns the token, false otherwise. - */ + /// @notice Validates basic token ownership. + /// @param subject Address to validate. + /// @param evidence Encoded tokenId. + /// @return Token ownership status. function _checkPre(address subject, bytes memory evidence) internal view override returns (bool) { super._checkPre(subject, evidence); - uint256 tokenId = abi.decode(evidence, (uint256)); return NFT.ownerOf(tokenId) == subject; } - /** - * @notice Main check verifies minimum token balance. - * @dev Validates if the subject holds at least MIN_BALANCE tokens. - * @param subject Address to check balance for. - * @param evidence Not used in this check. - * @return True if subject meets minimum balance requirement. - */ + /// @notice Validates minimum token balance. + /// @param subject Address to validate. + /// @param evidence Unused parameter. + /// @return Balance threshold status. function _checkMain(address subject, bytes memory evidence) internal view override returns (bool) { super._checkMain(subject, evidence); - return NFT.balanceOf(subject) >= MIN_BALANCE; } - /** - * @notice Post-check verifies ownership of a token within specific ID range. - * @dev Validates if subject owns a token with ID between MIN_TOKEN_ID and MAX_TOKEN_ID. - * @param subject Address to check ownership for. - * @param evidence Encoded uint256 tokenId. - * @return True if subject owns a token in valid range. - */ + /// @notice Validates token ID range ownership. + /// @param subject Address to validate. + /// @param evidence Encoded tokenId. + /// @return Token range validation status. function _checkPost(address subject, bytes memory evidence) internal view override returns (bool) { super._checkPost(subject, evidence); - uint256 tokenId = abi.decode(evidence, (uint256)); return tokenId >= MIN_TOKEN_ID && tokenId <= MAX_TOKEN_ID && NFT.ownerOf(tokenId) == subject; } diff --git a/packages/contracts/contracts/src/test/advanced/AdvancedERC721Policy.sol b/packages/contracts/contracts/src/test/advanced/AdvancedERC721Policy.sol index 1a488ca..f8cc5f5 100644 --- a/packages/contracts/contracts/src/test/advanced/AdvancedERC721Policy.sol +++ b/packages/contracts/contracts/src/test/advanced/AdvancedERC721Policy.sol @@ -4,22 +4,19 @@ pragma solidity 0.8.27; import {AdvancedPolicy} from "../../AdvancedPolicy.sol"; import {AdvancedERC721Checker} from "./AdvancedERC721Checker.sol"; -/** - * @title AdvancedERC721Policy - * @notice Policy contract implementing three-phase validation for ERC721 tokens. - * @dev Extends AdvancedPolicy to enforce ERC721-specific checks through AdvancedERC721Checker. - */ +/// @title AdvancedERC721Policy. +/// @notice Three-phase ERC721 validation policy. +/// @dev Enforces multi-stage checks through AdvancedERC721Checker. contract AdvancedERC721Policy is AdvancedPolicy { - /// @notice Reference to the ERC721 checker contract implementing validation logic. + /// @notice Checker contract reference. AdvancedERC721Checker public immutable CHECKER; - /// @param _checker Address of the AdvancedERC721Checker contract. + /// @notice Initializes with checker contract. constructor(AdvancedERC721Checker _checker) AdvancedPolicy(_checker) { CHECKER = _checker; } - /// @notice Returns the trait identifier for this policy. - /// @return String identifying this as an ERC721-based policy. + /// @notice Returns policy identifier. function trait() external pure returns (string memory) { return "AdvancedERC721"; } diff --git a/packages/contracts/contracts/src/test/advanced/AdvancedVoting.sol b/packages/contracts/contracts/src/test/advanced/AdvancedVoting.sol index eb3afa6..cabc41c 100644 --- a/packages/contracts/contracts/src/test/advanced/AdvancedVoting.sol +++ b/packages/contracts/contracts/src/test/advanced/AdvancedVoting.sol @@ -4,59 +4,72 @@ pragma solidity 0.8.27; import {AdvancedPolicy} from "../../AdvancedPolicy.sol"; import {Check} from "../../interfaces/IAdvancedPolicy.sol"; -/** - * @title AdvancedVoting - * @notice Voting contract with three-phase validation and NFT rewards. - * @dev Uses pre-check for registration, main check for voting, and post-check for claiming NFT rewards. - */ +/// @title AdvancedVoting. +/// @notice Multi-phase voting system with NFT validation and rewards. +/// @dev Implements a three-phase voting process: +/// 1. Registration (PRE): Validates initial NFT ownership. +/// 2. Voting (MAIN): Validates voting power and records vote. +/// 3. Rewards (POST): Validates and distributes NFT rewards. contract AdvancedVoting { + /// @notice Events for tracking system state changes. event Registered(address voter); event Voted(address voter, uint8 option); event RewardClaimed(address voter, uint256 rewardId); + /// @notice System error conditions. error NotRegistered(); error NotVoted(); error AlreadyClaimed(); error InvalidOption(); error NotOwnerOfReward(); - /// @notice Policy contract handling validation checks. + /// @notice Policy contract for validation checks. AdvancedPolicy public immutable POLICY; - /// @notice Tracks vote counts for each option. + /// @notice Tracks total votes per option. + /// @dev Maps option ID => vote count. mapping(uint8 => uint256) public voteCounts; + /// @notice Sets up voting system with policy contract. + /// @param _policy Contract handling validation logic. constructor(AdvancedPolicy _policy) { POLICY = _policy; } - /** - * @notice Register to participate in voting. - * @dev Validates NFT ownership through pre-check. - * @param tokenId Token ID to verify ownership. - */ + /// @notice First phase - Register voter with NFT ownership proof. + /// @dev Enforces PRE check through policy contract. + /// @param tokenId NFT used for registration. + /// @custom:requirements Caller must own the NFT with tokenId. + /// @custom:emits Registered on successful registration. function register(uint256 tokenId) external { + // Encode token ID for policy verification. bytes memory evidence = abi.encode(tokenId); + // Verify NFT ownership through policy's PRE check. POLICY.enforce(msg.sender, evidence, Check.PRE); emit Registered(msg.sender); } - /** - * @notice Cast vote after verifying registration. - * @dev Requires pre-check completion and validates voting power. - * @param option Voting option (0 or 1). - */ + /// @notice Second phase - Cast vote after registration. + /// @dev Enforces MAIN check and updates vote counts. + /// @param option Vote choice (0 or 1). + /// @custom:requirements + /// - Caller must be registered (passed PRE check). + /// - Option must be valid (0 or 1). + /// @custom:emits Voted on successful vote cast. function vote(uint8 option) external { + // Check registration status (PRE check completion). (bool pre, , ) = POLICY.enforced(address(this), msg.sender); if (!pre) revert NotRegistered(); if (option >= 2) revert InvalidOption(); + // Verify voting power through policy's MAIN check. bytes memory evidence = abi.encode(option); POLICY.enforce(msg.sender, evidence, Check.MAIN); + // Increment vote count safely. unchecked { voteCounts[option]++; } @@ -64,18 +77,23 @@ contract AdvancedVoting { emit Voted(msg.sender, option); } - /** - * @notice Claim NFT reward after voting. - * @dev Validates voting participation and transfers reward NFT. - * @param rewardId NFT ID to be claimed as reward. - */ + /// @notice Final phase - Claim NFT reward after voting. + /// @dev Enforces POST check for reward distribution. + /// @param rewardId Identifier of NFT reward to claim. + /// @custom:requirements + /// - Caller must be registered (passed PRE check). + /// - Caller must have voted (passed MAIN check). + /// - Caller must not have claimed before (no POST check). + /// @custom:emits RewardClaimed on successful claim. function reward(uint256 rewardId) external { + // Verify completion of previous phases. (bool pre, uint8 main, bool post) = POLICY.enforced(address(this), msg.sender); if (!pre) revert NotRegistered(); if (main == 0) revert NotVoted(); if (post) revert AlreadyClaimed(); + // Verify reward eligibility through policy's POST check. bytes memory evidence = abi.encode(rewardId); POLICY.enforce(msg.sender, evidence, Check.POST); diff --git a/packages/contracts/contracts/src/test/base/BaseERC721Checker.sol b/packages/contracts/contracts/src/test/base/BaseERC721Checker.sol index 5107569..9f9f0a2 100644 --- a/packages/contracts/contracts/src/test/base/BaseERC721Checker.sol +++ b/packages/contracts/contracts/src/test/base/BaseERC721Checker.sol @@ -4,32 +4,25 @@ pragma solidity 0.8.27; import {BaseChecker} from "../../../src/BaseChecker.sol"; import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; -/** - * @title BaseERC721Checker - * @notice Implements basic token ownership validation for ERC721 tokens. - * @dev Extends BaseChecker to provide simple ownership verification. - */ +/// @title BaseERC721Checker. +/// @notice ERC721 token ownership validator. +/// @dev Extends BaseChecker for NFT ownership verification. contract BaseERC721Checker is BaseChecker { + /// @notice NFT contract reference. IERC721 public immutable NFT; - /** - * @notice Initializes the checker with an ERC721 token contract. - * @param _nft Address of the ERC721 contract to check against. - */ + /// @notice Initializes with ERC721 contract. + /// @param _nft ERC721 contract address. constructor(IERC721 _nft) { NFT = IERC721(_nft); } - /** - * @notice Checks if the subject owns the specified token. - * @dev Validates if the subject is the owner of the tokenId provided in evidence. - * @param subject Address to check ownership for. - * @param evidence Encoded uint256 tokenId. - * @return True if subject owns the token, false otherwise. - */ + /// @notice Validates token ownership. + /// @param subject Address to check. + /// @param evidence Encoded tokenId. + /// @return True if subject owns token. function _check(address subject, bytes memory evidence) internal view override returns (bool) { super._check(subject, evidence); - uint256 tokenId = abi.decode(evidence, (uint256)); return NFT.ownerOf(tokenId) == subject; } diff --git a/packages/contracts/contracts/src/test/base/BaseERC721Policy.sol b/packages/contracts/contracts/src/test/base/BaseERC721Policy.sol index ecf9f55..ca99652 100644 --- a/packages/contracts/contracts/src/test/base/BaseERC721Policy.sol +++ b/packages/contracts/contracts/src/test/base/BaseERC721Policy.sol @@ -4,20 +4,21 @@ pragma solidity 0.8.27; import {BasePolicy} from "../../../src/BasePolicy.sol"; import {BaseERC721Checker} from "./BaseERC721Checker.sol"; -/** - * @title BaseERC721Policy - * @notice Policy contract for basic ERC721 token validation. - * @dev Extends BasePolicy to enforce NFT ownership checks. - */ +/// @title BaseERC721Policy. +/// @notice Policy enforcer for ERC721 token validation. +/// @dev Extends BasePolicy with NFT-specific checks. contract BaseERC721Policy is BasePolicy { - /// @notice Reference to the checker contract for token validation. + /// @notice Checker contract reference. BaseERC721Checker public immutable CHECKER; + /// @notice Initializes with checker contract. + /// @param _checker Checker contract address. constructor(BaseERC721Checker _checker) BasePolicy(_checker) { CHECKER = BaseERC721Checker(_checker); } - /// @notice Returns the trait identifier for this policy. + /// @notice Returns policy identifier. + /// @return Policy trait string. function trait() external pure returns (string memory) { return "BaseERC721"; } diff --git a/packages/contracts/contracts/src/test/base/BaseVoting.sol b/packages/contracts/contracts/src/test/base/BaseVoting.sol index 30476ca..79ebb0c 100644 --- a/packages/contracts/contracts/src/test/base/BaseVoting.sol +++ b/packages/contracts/contracts/src/test/base/BaseVoting.sol @@ -3,48 +3,68 @@ pragma solidity 0.8.27; import {BaseERC721Policy} from "./BaseERC721Policy.sol"; -/** - * @title BaseVoting - * @notice Basic voting contract with NFT-based access control. - * @dev Uses BaseERC721Policy for voter validation. - */ +/// @title BaseVoting. +/// @notice Simple voting system with NFT-based access control. +/// @dev Implements basic voting functionality with two phases: +/// 1. Registration: Validates NFT ownership. +/// 2. Voting: Records validated votes. contract BaseVoting { + /// @notice Emitted on successful registration/voting. event Registered(address voter); event Voted(address voter, uint8 option); + /// @notice System error conditions. error NotRegistered(); error AlreadyVoted(); error InvalidOption(); - /// @notice Policy contract for voter validation. + /// @notice Policy contract for NFT validation. BaseERC721Policy public immutable POLICY; - /// @notice Tracks if an address has voted. + /// @dev Maps voter address => voting status. mapping(address => bool) public hasVoted; - /// @notice Counts votes for each option. + /// @dev Maps option ID => vote count. mapping(uint8 => uint256) public voteCounts; + /// @notice Sets up voting system. + /// @param _policy Contract for voter validation. constructor(BaseERC721Policy _policy) { POLICY = _policy; } - /// @notice Register voter using NFT ownership verification. - /// @param tokenId Token ID to verify ownership. + /// @notice Register using NFT ownership proof. + /// @dev Enforces NFT ownership check through policy. + /// @param tokenId NFT used for registration. + /// @custom:requirements Caller must own the NFT with tokenId. + /// @custom:emits Registered on successful registration. function register(uint256 tokenId) external { + // Encode token ID for policy verification. bytes memory evidence = abi.encode(tokenId); + + // Verify NFT ownership. POLICY.enforce(msg.sender, evidence); + emit Registered(msg.sender); } - /// @notice Cast vote for given option. - /// @param option Voting option (0 or 1). + /// @notice Cast vote after registration. + /// @dev Updates vote counts if validation passes. + /// @param option Vote choice (0 or 1). + /// @custom:requirements + /// - Caller must be registered. + /// - Caller must not have voted. + /// - Option must be valid (0 or 1). + /// @custom:emits Voted on successful vote cast. function vote(uint8 option) external { + // Verify registration and voting status. if (!POLICY.enforced(address(this), msg.sender)) revert NotRegistered(); if (hasVoted[msg.sender]) revert AlreadyVoted(); if (option >= 2) revert InvalidOption(); + // Record vote. hasVoted[msg.sender] = true; voteCounts[option]++; + emit Voted(msg.sender, option); } } diff --git a/packages/contracts/contracts/src/test/utils/NFT.sol b/packages/contracts/contracts/src/test/utils/NFT.sol index 183d16d..d631f5b 100644 --- a/packages/contracts/contracts/src/test/utils/NFT.sol +++ b/packages/contracts/contracts/src/test/utils/NFT.sol @@ -3,11 +3,17 @@ pragma solidity ^0.8.0; import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol"; +/// @title NFT. +/// @notice Simple ERC721 implementation for testing. contract NFT is ERC721 { + /// @dev Tracks the next token ID to mint. uint256 private _tokenIdCounter; + /// @notice Initializes NFT with name and symbol "NFT". constructor() ERC721("NFT", "NFT") {} + /// @notice Mints new token to specified address. + /// @param to Recipient address. function mint(address to) external { _safeMint(to, _tokenIdCounter); _tokenIdCounter++; diff --git a/packages/contracts/contracts/src/test/wrappers/AdvancedERC721CheckerHarness.sol b/packages/contracts/contracts/src/test/wrappers/AdvancedERC721CheckerHarness.sol index 38a73be..bfd4630 100644 --- a/packages/contracts/contracts/src/test/wrappers/AdvancedERC721CheckerHarness.sol +++ b/packages/contracts/contracts/src/test/wrappers/AdvancedERC721CheckerHarness.sol @@ -5,9 +5,18 @@ import {AdvancedERC721Checker} from "../advanced/AdvancedERC721Checker.sol"; import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import {Check} from "../../interfaces/IAdvancedChecker.sol"; -// This contract is a harness for testing the AdvancedERC721Checker contract. -// Deploy this contract and call its methods to test the internal methods of AdvancedERC721Checker. +/// @title AdvancedERC721CheckerHarness. +/// @notice Test harness exposing internal methods of AdvancedERC721Checker. +/// @dev Inherits AdvancedERC721Checker and exposes protected methods for testing. contract AdvancedERC721CheckerHarness is AdvancedERC721Checker { + /// @notice Initializes test harness with checker configuration. + /// @param _nft ERC721 contract to validate. + /// @param _minBalance Minimum token balance required. + /// @param _minTokenId Minimum valid token ID. + /// @param _maxTokenId Maximum valid token ID. + /// @param _skipPre Skip pre-check validation. + /// @param _skipPost Skip post-check validation. + /// @param _allowMultipleMain Allow multiple main validations. constructor( IERC721 _nft, uint256 _minBalance, @@ -18,31 +27,35 @@ contract AdvancedERC721CheckerHarness is AdvancedERC721Checker { bool _allowMultipleMain ) AdvancedERC721Checker(_nft, _minBalance, _minTokenId, _maxTokenId, _skipPre, _skipPost, _allowMultipleMain) {} - /// @notice Exposes the internal `_check` method for testing purposes. - /// @param subject The address to be checked. - /// @param evidence The data associated with the check. - /// @param checkType The type of check to perform (PRE, MAIN, POST). + /// @notice Test exposure for _check method. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @param checkType Type of check to perform. + /// @return Validation result. function exposed__check(address subject, bytes calldata evidence, Check checkType) public view returns (bool) { return _check(subject, evidence, checkType); } - /// @notice Exposes the internal `_checkPre` method for testing purposes. - /// @param subject The address to be checked. - /// @param evidence The data associated with the check. + /// @notice Test exposure for _checkPre method. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @return Pre-check validation result. function exposed__checkPre(address subject, bytes calldata evidence) public view returns (bool) { return _checkPre(subject, evidence); } - /// @notice Exposes the internal `_checkMain` method for testing purposes. - /// @param subject The address to be checked. - /// @param evidence The data associated with the check. + /// @notice Test exposure for _checkMain method. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @return Main validation result. function exposed__checkMain(address subject, bytes calldata evidence) public view returns (bool) { return _checkMain(subject, evidence); } - /// @notice Exposes the internal `_checkPost` method for testing purposes. - /// @param subject The address to be checked. - /// @param evidence The data associated with the check. + /// @notice Test exposure for _checkPost method. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @return Post-check validation result. function exposed__checkPost(address subject, bytes calldata evidence) public view returns (bool) { return _checkPost(subject, evidence); } diff --git a/packages/contracts/contracts/src/test/wrappers/AdvancedERC721PolicyHarness.sol b/packages/contracts/contracts/src/test/wrappers/AdvancedERC721PolicyHarness.sol index e31d760..9abea4e 100644 --- a/packages/contracts/contracts/src/test/wrappers/AdvancedERC721PolicyHarness.sol +++ b/packages/contracts/contracts/src/test/wrappers/AdvancedERC721PolicyHarness.sol @@ -5,15 +5,16 @@ import {AdvancedERC721Policy} from "../advanced/AdvancedERC721Policy.sol"; import {AdvancedERC721Checker} from "../advanced/AdvancedERC721Checker.sol"; import {Check} from "../../interfaces/IAdvancedChecker.sol"; -// This contract is a harness for testing the AdvancedERC721Policy contract. -// Deploy this contract and call its methods to test the internal methods of AdvancedERC721Policy. +/// @title AdvancedERC721PolicyHarness. +/// @notice Test harness for AdvancedERC721Policy internal methods. contract AdvancedERC721PolicyHarness is AdvancedERC721Policy { + /// @notice Initializes test harness. constructor(AdvancedERC721Checker _checker) AdvancedERC721Policy(_checker) {} - /// @notice Exposes the internal `_enforce` method for testing purposes. - /// @param subject The address of those who have successfully enforced the check. - /// @param evidence Additional data required for the check (e.g., encoded token identifier). - /// @param checkType The type of the check to be enforced for the subject with the given data. + /// @notice Test exposure for _enforce method. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @param checkType Check type to enforce. function exposed__enforce(address subject, bytes calldata evidence, Check checkType) public onlyTarget { _enforce(subject, evidence, checkType); } diff --git a/packages/contracts/contracts/src/test/wrappers/BaseERC721CheckerHarness.sol b/packages/contracts/contracts/src/test/wrappers/BaseERC721CheckerHarness.sol index dce0d8e..399126c 100644 --- a/packages/contracts/contracts/src/test/wrappers/BaseERC721CheckerHarness.sol +++ b/packages/contracts/contracts/src/test/wrappers/BaseERC721CheckerHarness.sol @@ -4,14 +4,16 @@ pragma solidity 0.8.27; import {BaseERC721Checker} from "../base/BaseERC721Checker.sol"; import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; -// This contract is a harness for testing the BaseERC721Checker contract. -// Deploy this contract and call its methods to test the internal methods of BaseERC721Checker. +/// @title BaseERC721CheckerHarness. +/// @notice Test harness for BaseERC721Checker internal methods. contract BaseERC721CheckerHarness is BaseERC721Checker { + /// @notice Initializes test harness with NFT contract. constructor(IERC721 _nft) BaseERC721Checker(_nft) {} - /// @notice Exposes the internal `_check` method for testing purposes. - /// @param subject The address to be checked. - /// @param evidence The data associated with the check. + /// @notice Test exposure for _check method. + /// @param subject Address to validate. + /// @param evidence Validation data. + /// @return Validation result. function exposed__check(address subject, bytes calldata evidence) public view returns (bool) { return _check(subject, evidence); } diff --git a/packages/contracts/contracts/src/test/wrappers/BaseERC721PolicyHarness.sol b/packages/contracts/contracts/src/test/wrappers/BaseERC721PolicyHarness.sol index 8816bb6..2e96d77 100644 --- a/packages/contracts/contracts/src/test/wrappers/BaseERC721PolicyHarness.sol +++ b/packages/contracts/contracts/src/test/wrappers/BaseERC721PolicyHarness.sol @@ -4,14 +4,15 @@ pragma solidity 0.8.27; import {BaseERC721Policy} from "../base/BaseERC721Policy.sol"; import {BaseERC721Checker} from "../base/BaseERC721Checker.sol"; -// This contract is a harness for testing the BaseERC721Policy contract. -// Deploy this contract and call its methods to test the internal methods of BaseERC721Policy. +/// @title BaseERC721PolicyHarness. +/// @notice Test harness for BaseERC721Policy internal methods. contract BaseERC721PolicyHarness is BaseERC721Policy { + /// @notice Initializes test harness with checker. constructor(BaseERC721Checker _checker) BaseERC721Policy(_checker) {} - /// @notice Exposes the internal `_enforce` method for testing purposes. - /// @param subject The address to be checked. - /// @param evidence The data associated with the check. + /// @notice Test exposure for _enforce method. + /// @param subject Address to validate. + /// @param evidence Validation data. function exposed__enforce(address subject, bytes calldata evidence) public onlyTarget { _enforce(subject, evidence); } diff --git a/packages/contracts/test/Advanced.test.ts b/packages/contracts/test/Advanced.test.ts index f681bef..e2f7df6 100644 --- a/packages/contracts/test/Advanced.test.ts +++ b/packages/contracts/test/Advanced.test.ts @@ -68,17 +68,17 @@ describe("Advanced", () => { } } - describe("constructor()", () => { - it("Should deploy the checker contract correctly", async () => { + describe("constructor", () => { + it("deploys correctly", async () => { const { checker } = await loadFixture(deployAdvancedCheckerFixture) expect(checker).to.not.eq(undefined) }) }) - describe("check()", () => { - describe("pre", () => { - it("should revert the check when the evidence is not meaningful", async () => { + describe("check", () => { + describe("pre check", () => { + it("reverts when evidence invalid", async () => { const { nft, checker, target, subjectAddress, invalidNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -87,37 +87,37 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(nft, "ERC721NonexistentToken") }) - it("should return false when the subject is not the owner of the evidenced token", async () => { + it("returns false when not owner", async () => { const { checker, target, notOwnerAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) expect(await checker.connect(target).check(notOwnerAddress, validNFTId, 0)).to.be.equal(false) }) - it("should check", async () => { + it("succeeds when valid", async () => { const { checker, target, subjectAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) expect(await checker.connect(target).check(subjectAddress, validNFTId, 0)).to.be.equal(true) }) }) - describe("main", () => { - it("should return false when the subject does not satisfy the attributes", async () => { + describe("main check", () => { + it("returns false when balance insufficient", async () => { const { checker, target, notOwnerAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) expect(await checker.connect(target).check(notOwnerAddress, validNFTId, 1)).to.be.equal(false) }) - it("should check", async () => { + it("succeeds when balance sufficient", async () => { const { checker, target, subjectAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) expect(await checker.connect(target).check(subjectAddress, validNFTId, 1)).to.be.equal(true) }) }) - describe("post", () => { - it("should revert the check when the evidence is not meaningful", async () => { + describe("post check", () => { + it("reverts when evidence invalid", async () => { const { nft, checker, target, subjectAddress, invalidNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -126,14 +126,14 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(nft, "ERC721NonexistentToken") }) - it("should return false when the subject does not satisfy the attributes", async () => { + it("returns false when not in range", async () => { const { checker, target, notOwnerAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) expect(await checker.connect(target).check(notOwnerAddress, validNFTId, 2)).to.be.equal(false) }) - it("should check", async () => { + it("succeeds when in valid range", async () => { const { checker, target, subjectAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -142,9 +142,9 @@ describe("Advanced", () => { }) }) - describe("_check()", () => { - describe("_pre", () => { - it("should revert the check when the evidence is not meaningful", async () => { + describe("internal checks", () => { + describe("pre check", () => { + it("reverts when evidence invalid", async () => { const { nft, checkerHarness, target, subjectAddress, invalidNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -153,7 +153,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(nft, "ERC721NonexistentToken") }) - it("should return false when the subject is not the owner of the evidenced token", async () => { + it("returns false when not owner", async () => { const { checkerHarness, target, notOwnerAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -162,7 +162,7 @@ describe("Advanced", () => { ).to.be.equal(false) }) - it("should check", async () => { + it("succeeds when valid", async () => { const { checkerHarness, target, subjectAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -171,8 +171,8 @@ describe("Advanced", () => { ).to.be.equal(true) }) }) - describe("_main", () => { - it("should return false when the subject does not satisfy the attributes", async () => { + describe("main check", () => { + it("returns false when balance insufficient", async () => { const { checkerHarness, target, notOwnerAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -181,7 +181,7 @@ describe("Advanced", () => { ).to.be.equal(false) }) - it("should check", async () => { + it("succeeds when balance sufficient", async () => { const { checkerHarness, target, subjectAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -190,8 +190,8 @@ describe("Advanced", () => { ).to.be.equal(true) }) }) - describe("_post", () => { - it("should revert the check when the evidence is not meaningful", async () => { + describe("post check", () => { + it("reverts when evidence invalid", async () => { const { nft, checkerHarness, target, subjectAddress, invalidNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -200,7 +200,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(nft, "ERC721NonexistentToken") }) - it("should return false when the subject does not satisfy the attributes", async () => { + it("returns false when not in range", async () => { const { checkerHarness, target, notOwnerAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -209,7 +209,7 @@ describe("Advanced", () => { ).to.be.equal(false) }) - it("should check", async () => { + it("succeeds when in valid range", async () => { const { checkerHarness, target, subjectAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -220,8 +220,8 @@ describe("Advanced", () => { }) }) - describe("_checkPre", () => { - it("should revert the check when the evidence is not meaningful", async () => { + describe("internal checkPre", () => { + it("reverts when evidence invalid", async () => { const { nft, checkerHarness, target, subjectAddress, invalidNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -230,7 +230,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(nft, "ERC721NonexistentToken") }) - it("should return false when the subject is not the owner of the evidenced token", async () => { + it("returns false when not owner", async () => { const { checkerHarness, target, notOwnerAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -239,7 +239,7 @@ describe("Advanced", () => { ) }) - it("should check", async () => { + it("succeeds when valid", async () => { const { checkerHarness, target, subjectAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -249,8 +249,8 @@ describe("Advanced", () => { }) }) - describe("_checkMain", () => { - it("should return false when the subject does not satisfy the attributes", async () => { + describe("internal checkMain", () => { + it("returns false when balance insufficient", async () => { const { checkerHarness, target, notOwnerAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -259,7 +259,7 @@ describe("Advanced", () => { ).to.be.equal(false) }) - it("should check", async () => { + it("succeeds when balance sufficient", async () => { const { checkerHarness, target, subjectAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -269,8 +269,8 @@ describe("Advanced", () => { }) }) - describe("_checkPost", () => { - it("should revert the check when the evidence is not meaningful", async () => { + describe("internal checkPost", () => { + it("reverts when evidence invalid", async () => { const { nft, checkerHarness, target, subjectAddress, invalidNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -279,7 +279,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(nft, "ERC721NonexistentToken") }) - it("should return false when the subject does not satisfy the attributes", async () => { + it("returns false when not in range", async () => { const { checkerHarness, target, notOwnerAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -288,7 +288,7 @@ describe("Advanced", () => { ).to.be.equal(false) }) - it("should check", async () => { + it("succeeds when in valid range", async () => { const { checkerHarness, target, subjectAddress, validNFTId } = await loadFixture(deployAdvancedCheckerFixture) @@ -370,24 +370,24 @@ describe("Advanced", () => { } } - describe("constructor()", () => { - it("Should deploy the checker contract correctly", async () => { + describe("constructor", () => { + it("deploys correctly", async () => { const { policy } = await loadFixture(deployAdvancedPolicyFixture) expect(policy).to.not.eq(undefined) }) }) - describe("trait()", () => { - it("should return the trait of the policy contract", async () => { + describe("trait", () => { + it("returns correct value", async () => { const { policy } = await loadFixture(deployAdvancedPolicyFixture) expect(await policy.trait()).to.be.eq("AdvancedERC721") }) }) - describe("setTarget()", () => { - it("should fail to set the target when the caller is not the owner", async () => { + describe("setTarget", () => { + it("reverts when caller not owner", async () => { const { policy, notOwner, target } = await loadFixture(deployAdvancedPolicyFixture) await expect( @@ -395,7 +395,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policy, "OwnableUnauthorizedAccount") }) - it("should fail to set the target when the target address is zero", async () => { + it("reverts when zero address", async () => { const { policy, deployer } = await loadFixture(deployAdvancedPolicyFixture) await expect(policy.connect(deployer).setTarget(ZeroAddress)).to.be.revertedWithCustomError( @@ -404,7 +404,7 @@ describe("Advanced", () => { ) }) - it("Should set the target contract address correctly", async () => { + it("sets target correctly", async () => { const { policy, target, AdvancedERC721PolicyFactory } = await loadFixture(deployAdvancedPolicyFixture) const targetAddress = await target.getAddress() @@ -423,7 +423,7 @@ describe("Advanced", () => { expect(await policy.getTarget()).to.eq(targetAddress) }) - it("Should fail to set the target if already set", async () => { + it("reverts when already set", async () => { const { policy, target } = await loadFixture(deployAdvancedPolicyFixture) const targetAddress = await target.getAddress() @@ -433,9 +433,9 @@ describe("Advanced", () => { }) }) - describe("enforce()", () => { - describe("pre", () => { - it("should throw when the callee is not the target", async () => { + describe("enforce", () => { + describe("pre check", () => { + it("reverts when caller not target", async () => { const { policy, subject, target, subjectAddress } = await loadFixture(deployAdvancedPolicyFixture) await policy.setTarget(await target.getAddress()) @@ -445,7 +445,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policy, "TargetOnly") }) - it("should throw when the evidence is not correct", async () => { + it("reverts when evidence invalid", async () => { const { iERC721Errors, policy, target, subjectAddress, invalidEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -456,7 +456,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(iERC721Errors, "ERC721NonexistentToken") }) - it("should throw when the check is skipped", async () => { + it("reverts when pre-check skipped", async () => { const { checker, policySkipped, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -467,7 +467,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(checker, "PreCheckSkipped") }) - it("should throw when the check returns false", async () => { + it("reverts when check unsuccessful", async () => { const { policy, target, notOwnerAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -478,7 +478,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policy, "UnsuccessfulCheck") }) - it("should enforce", async () => { + it("enforces pre-check successfully", async () => { const { AdvancedERC721PolicyFactory, policy, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) const targetAddress = await target.getAddress() @@ -506,7 +506,7 @@ describe("Advanced", () => { expect((await policy.enforced(targetAddress, subjectAddress))[0]).to.be.equal(true) }) - it("should prevent to enforce twice", async () => { + it("reverts when pre already enforced", async () => { const { policy, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -520,8 +520,8 @@ describe("Advanced", () => { }) }) - describe("main", () => { - it("should throw when the subject does not satisfy the chain of checks", async () => { + describe("main check", () => { + it("reverts when pre-check missing", async () => { const { policy, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -532,7 +532,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policy, "PreCheckNotEnforced") }) - it("should throw when the subject does not satisfy the attributes", async () => { + it("reverts when check unsuccessful", async () => { const { policy, target, notOwnerAddress, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -544,7 +544,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policy, "UnsuccessfulCheck") }) - it("should enforce", async () => { + it("enforces main-check successfully", async () => { const { AdvancedERC721PolicyFactory, policy, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) const targetAddress = await target.getAddress() @@ -573,7 +573,7 @@ describe("Advanced", () => { expect((await policy.enforced(targetAddress, subjectAddress))[1]).to.be.equal(1) }) - it("should enforce twice when allowed", async () => { + it("executes multiple mains when allowed", async () => { const { AdvancedERC721PolicyFactory, policy, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) const targetAddress = await target.getAddress() @@ -603,7 +603,7 @@ describe("Advanced", () => { expect((await policy.enforced(targetAddress, subjectAddress))[1]).to.be.equal(2) }) - it("should prevent to enforce twice when not allowed", async () => { + it("executes multiple mains when allowed", async () => { const { policySkipped, target, notOwnerAddress, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -616,8 +616,8 @@ describe("Advanced", () => { }) }) - describe("post", () => { - it("should throw when the subject does not satisfy the chain of checks", async () => { + describe("post check", () => { + it("reverts when pre/main missing", async () => { const { policy, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -634,7 +634,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policy, "MainCheckNotEnforced") }) - it("should throw when the callee is not the target", async () => { + it("reverts when caller not target", async () => { const { policy, subject, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -647,7 +647,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policy, "TargetOnly") }) - it("should throw when the evidence is not correct", async () => { + it("reverts when evidence invalid", async () => { const { iERC721Errors, policy, target, subjectAddress, validEncodedNFTId, invalidEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -660,7 +660,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(iERC721Errors, "ERC721NonexistentToken") }) - it("should throw when the check is skipped", async () => { + it("reverts when post-check skipped", async () => { const { checker, policySkipped, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -672,7 +672,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(checker, "PostCheckSkipped") }) - it("should throw when the check returns false", async () => { + it("reverts when check unsuccessful", async () => { const { policy, target, subjectAddress, notOwnerAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -685,7 +685,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policy, "UnsuccessfulCheck") }) - it("should enforce", async () => { + it("enforces post-check successfully", async () => { const { AdvancedERC721PolicyFactory, policy, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) const targetAddress = await target.getAddress() @@ -715,7 +715,7 @@ describe("Advanced", () => { expect((await policy.enforced(targetAddress, subjectAddress))[2]).to.be.equal(true) }) - it("should prevent to enforce twice", async () => { + it("reverts when post already enforced", async () => { const { policy, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -731,9 +731,9 @@ describe("Advanced", () => { }) }) - describe("_enforce()", () => { - describe("_pre", () => { - it("should throw when the callee is not the target", async () => { + describe("internal enforce", () => { + describe("internal pre", () => { + it("reverts when caller not target", async () => { const { policyHarness, subject, target, subjectAddress } = await loadFixture(deployAdvancedPolicyFixture) @@ -744,7 +744,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policyHarness, "TargetOnly") }) - it("should throw when the evidence is not correct", async () => { + it("reverts when evidence invalid", async () => { const { iERC721Errors, policyHarness, target, subjectAddress, invalidEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -755,7 +755,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(iERC721Errors, "ERC721NonexistentToken") }) - it("should throw when the check is skipped", async () => { + it("reverts when pre-check skipped", async () => { const { checker, policyHarnessSkipped, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -766,7 +766,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(checker, "PreCheckSkipped") }) - it("should throw when the check returns false", async () => { + it("reverts when check unsuccessful", async () => { const { policyHarness, target, notOwnerAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -777,7 +777,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policyHarness, "UnsuccessfulCheck") }) - it("should enforce", async () => { + it("enforces pre-check successfully", async () => { const { AdvancedERC721PolicyFactory, policyHarness, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) const targetAddress = await target.getAddress() @@ -805,7 +805,7 @@ describe("Advanced", () => { expect((await policyHarness.enforced(targetAddress, subjectAddress))[0]).to.be.equal(true) }) - it("should prevent to enforce twice", async () => { + it("reverts when pre already enforced", async () => { const { policyHarness, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -820,7 +820,7 @@ describe("Advanced", () => { }) describe("_main", () => { - it("should throw when the subject does not satisfy the chain of checks", async () => { + it("reverts when pre-check missing", async () => { const { policyHarness, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -831,7 +831,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policyHarness, "PreCheckNotEnforced") }) - it("should throw when the subject does not satisfy the attributes", async () => { + it("reverts when check unsuccessful", async () => { const { policyHarness, target, notOwnerAddress, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -843,7 +843,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policyHarness, "UnsuccessfulCheck") }) - it("should enforce", async () => { + it("enforces main-check successfully", async () => { const { AdvancedERC721PolicyFactory, policyHarness, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) const targetAddress = await target.getAddress() @@ -872,7 +872,7 @@ describe("Advanced", () => { expect((await policyHarness.enforced(targetAddress, subjectAddress))[1]).to.be.equal(1) }) - it("should enforce twice when allowed", async () => { + it("executes multiple mains when allowed", async () => { const { AdvancedERC721PolicyFactory, policyHarness, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) const targetAddress = await target.getAddress() @@ -902,7 +902,7 @@ describe("Advanced", () => { expect((await policyHarness.enforced(targetAddress, subjectAddress))[1]).to.be.equal(2) }) - it("should prevent to enforce twice when not allowed", async () => { + it("executes multiple mains when allowed", async () => { const { policyHarnessSkipped, target, notOwnerAddress, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -919,7 +919,7 @@ describe("Advanced", () => { }) describe("_post", () => { - it("should throw when the subject does not satisfy the chain of checks", async () => { + it("reverts when pre/main missing", async () => { const { policyHarness, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -936,7 +936,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policyHarness, "MainCheckNotEnforced") }) - it("should throw when the callee is not the target", async () => { + it("reverts when caller not target", async () => { const { policyHarness, subject, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -949,7 +949,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policyHarness, "TargetOnly") }) - it("should throw when the evidence is not correct", async () => { + it("reverts when evidence invalid", async () => { const { iERC721Errors, policyHarness, @@ -968,7 +968,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(iERC721Errors, "ERC721NonexistentToken") }) - it("should throw when the check is skipped", async () => { + it("reverts when post-check skipped", async () => { const { checker, policyHarnessSkipped, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -980,7 +980,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(checker, "PostCheckSkipped") }) - it("should throw when the check returns false", async () => { + it("reverts when check unsuccessful", async () => { const { policyHarness, target, subjectAddress, notOwnerAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -993,7 +993,7 @@ describe("Advanced", () => { ).to.be.revertedWithCustomError(policyHarness, "UnsuccessfulCheck") }) - it("should enforce", async () => { + it("enforces post-check successfully", async () => { const { AdvancedERC721PolicyFactory, policyHarness, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) const targetAddress = await target.getAddress() @@ -1023,7 +1023,7 @@ describe("Advanced", () => { expect((await policyHarness.enforced(targetAddress, subjectAddress))[2]).to.be.equal(true) }) - it("should prevent to enforce twice", async () => { + it("reverts when post already enforced", async () => { const { policyHarness, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployAdvancedPolicyFixture) @@ -1101,16 +1101,16 @@ describe("Advanced", () => { } } - describe("constructor()", () => { - it("Should deploy the voting contract correctly", async () => { + describe("constructor", () => { + it("deploys correctly", async () => { const { voting } = await loadFixture(deployAdvancedVotingFixture) expect(voting).to.not.eq(undefined) }) }) - describe("register()", () => { - it("Should revert when the callee is not the target", async () => { + describe("register", () => { + it("reverts when caller not target", async () => { const { voting, policy, notOwner, validNFTId } = await loadFixture(deployAdvancedVotingFixture) await policy.setTarget(await notOwner.getAddress()) @@ -1121,7 +1121,7 @@ describe("Advanced", () => { ) }) - it("Should revert when the evidence is not correct", async () => { + it("reverts when evidence invalid", async () => { const { iERC721Errors, voting, policy, subject, invalidNFTId } = await loadFixture(deployAdvancedVotingFixture) @@ -1133,7 +1133,7 @@ describe("Advanced", () => { ) }) - it("should throw when the registration check returns false", async () => { + it("reverts when check fails", async () => { const { voting, policy, notOwner, validNFTId } = await loadFixture(deployAdvancedVotingFixture) await policy.setTarget(await voting.getAddress()) @@ -1144,7 +1144,7 @@ describe("Advanced", () => { ) }) - it("should register", async () => { + it("registers successfully", async () => { const { AdvancedVotingFactory, voting, policy, subject, validNFTId, subjectAddress } = await loadFixture(deployAdvancedVotingFixture) const targetAddress = await voting.getAddress() @@ -1169,7 +1169,7 @@ describe("Advanced", () => { expect(await voting.voteCounts(1)).to.be.equal(0) }) - it("should prevent to register twice", async () => { + it("reverts when already registered", async () => { const { voting, policy, subject, validNFTId } = await loadFixture(deployAdvancedVotingFixture) const targetAddress = await voting.getAddress() @@ -1184,8 +1184,8 @@ describe("Advanced", () => { }) }) - describe("vote()", () => { - it("Should revert when the callee is not registered", async () => { + describe("vote", () => { + it("reverts when not registered", async () => { const { voting, policy, subject } = await loadFixture(deployAdvancedVotingFixture) await policy.setTarget(await voting.getAddress()) @@ -1193,7 +1193,7 @@ describe("Advanced", () => { await expect(voting.connect(subject).vote(0)).to.be.revertedWithCustomError(voting, "NotRegistered") }) - it("Should revert when the option is not correct", async () => { + it("reverts when option invalid", async () => { const { voting, policy, subject, validNFTId } = await loadFixture(deployAdvancedVotingFixture) await policy.setTarget(await voting.getAddress()) @@ -1202,7 +1202,7 @@ describe("Advanced", () => { await expect(voting.connect(subject).vote(3)).to.be.revertedWithCustomError(voting, "InvalidOption") }) - it("should vote", async () => { + it("votes successfully", async () => { const { AdvancedVotingFactory, voting, policy, subject, subjectAddress, validNFTId } = await loadFixture(deployAdvancedVotingFixture) const option = 0 @@ -1231,7 +1231,7 @@ describe("Advanced", () => { expect(await voting.voteCounts(1)).to.be.equal(0) }) - it("should vote twice", async () => { + it("allows multiple votes", async () => { const { AdvancedVotingFactory, voting, policy, subject, subjectAddress, validNFTId } = await loadFixture(deployAdvancedVotingFixture) const option = 0 @@ -1262,8 +1262,8 @@ describe("Advanced", () => { }) }) - describe("reward()", () => { - it("Should revert when the callee is not the target", async () => { + describe("reward", () => { + it("reverts when caller not target", async () => { const { voting, policy, subject, notOwner, validNFTId } = await loadFixture(deployAdvancedVotingFixture) await policy.setTarget(await notOwner.getAddress()) @@ -1274,7 +1274,7 @@ describe("Advanced", () => { ) }) - it("Should revert when the evidence is not correct", async () => { + it("reverts when evidence invalid", async () => { const { iERC721Errors, voting, policy, subject, validNFTId, invalidNFTId } = await loadFixture(deployAdvancedVotingFixture) @@ -1288,7 +1288,7 @@ describe("Advanced", () => { ) }) - it("should throw when the rewared check returns false", async () => { + it("reverts when check fails", async () => { const { nft, deployer, voting, policy, notOwner, subject, validNFTId } = await loadFixture(deployAdvancedVotingFixture) @@ -1305,7 +1305,7 @@ describe("Advanced", () => { ) }) - it("Should revert when the callee is not registered", async () => { + it("reverts when not registered", async () => { const { voting, policy, notOwner, validNFTId } = await loadFixture(deployAdvancedVotingFixture) await policy.setTarget(await notOwner.getAddress()) @@ -1316,7 +1316,7 @@ describe("Advanced", () => { ) }) - it("Should revert when the callee has not voted", async () => { + it("reverts when not voted", async () => { const { voting, policy, subject, validNFTId } = await loadFixture(deployAdvancedVotingFixture) await policy.setTarget(await voting.getAddress()) @@ -1328,7 +1328,7 @@ describe("Advanced", () => { ) }) - it("should get the reward", async () => { + it("claims reward successfully", async () => { const { AdvancedVotingFactory, voting, policy, subject, subjectAddress, validNFTId } = await loadFixture(deployAdvancedVotingFixture) const targetAddress = await voting.getAddress() @@ -1356,7 +1356,7 @@ describe("Advanced", () => { expect(await voting.voteCounts(1)).to.be.equal(0) }) - it("should prevent to reward twice", async () => { + it("reverts when already claimed", async () => { const { voting, policy, subject, validNFTId } = await loadFixture(deployAdvancedVotingFixture) await policy.setTarget(await voting.getAddress()) @@ -1370,8 +1370,8 @@ describe("Advanced", () => { ) }) }) - describe("e2e", () => { - it("should submit a vote for each subject", async () => { + describe("end to end", () => { + it("completes full voting lifecycle", async () => { const [deployer]: Signer[] = await ethers.getSigners() const NFTFactory: NFT__factory = await ethers.getContractFactory("NFT") diff --git a/packages/contracts/test/Base.test.ts b/packages/contracts/test/Base.test.ts index 4960bae..bcf7cb3 100644 --- a/packages/contracts/test/Base.test.ts +++ b/packages/contracts/test/Base.test.ts @@ -58,16 +58,16 @@ describe("Base", () => { } } - describe("constructor()", () => { - it("Should deploy the checker contract correctly", async () => { + describe("constructor", () => { + it("deploys correctly", async () => { const { checker } = await loadFixture(deployBaseCheckerFixture) expect(checker).to.not.eq(undefined) }) }) - describe("check()", () => { - it("should revert the check when the evidence is not meaningful", async () => { + describe("check", () => { + it("reverts when evidence is invalid", async () => { const { nft, checker, target, subjectAddress, invalidNFTId } = await loadFixture(deployBaseCheckerFixture) @@ -77,21 +77,21 @@ describe("Base", () => { ) }) - it("should return false when the subject is not the owner of the evidenced token", async () => { + it("returns false when subject not owner", async () => { const { checker, target, notOwnerAddress, validNFTId } = await loadFixture(deployBaseCheckerFixture) expect(await checker.connect(target).check(notOwnerAddress, validNFTId)).to.be.equal(false) }) - it("should check", async () => { + it("succeeds when valid", async () => { const { checker, target, subjectAddress, validNFTId } = await loadFixture(deployBaseCheckerFixture) expect(await checker.connect(target).check(subjectAddress, validNFTId)).to.be.equal(true) }) }) - describe("_check()", () => { - it("should revert the check when the evidence is not meaningful", async () => { + describe("internal check", () => { + it("reverts when evidence is invalid", async () => { const { nft, checkerHarness, target, subjectAddress, invalidNFTId } = await loadFixture(deployBaseCheckerFixture) @@ -100,7 +100,7 @@ describe("Base", () => { ).to.be.revertedWithCustomError(nft, "ERC721NonexistentToken") }) - it("should return false when the subject is not the owner of the evidenced token", async () => { + it("returns false when subject not owner", async () => { const { checkerHarness, target, notOwnerAddress, validNFTId } = await loadFixture(deployBaseCheckerFixture) @@ -109,7 +109,7 @@ describe("Base", () => { ) }) - it("should check", async () => { + it("succeeds when valid", async () => { const { checkerHarness, target, subjectAddress, validNFTId } = await loadFixture(deployBaseCheckerFixture) @@ -171,24 +171,24 @@ describe("Base", () => { } } - describe("constructor()", () => { - it("Should deploy the policy contract correctly", async () => { + describe("constructor", () => { + it("deploys correctly", async () => { const { policy } = await loadFixture(deployBasePolicyFixture) expect(policy).to.not.eq(undefined) }) }) - describe("trait()", () => { - it("should return the trait of the policy contract", async () => { + describe("trait", () => { + it("returns correct value", async () => { const { policy } = await loadFixture(deployBasePolicyFixture) expect(await policy.trait()).to.be.eq("BaseERC721") }) }) - describe("setTarget()", () => { - it("should fail to set the target when the caller is not the owner", async () => { + describe("setTarget", () => { + it("reverts when caller not owner", async () => { const { policy, notOwner, target } = await loadFixture(deployBasePolicyFixture) await expect( @@ -196,7 +196,7 @@ describe("Base", () => { ).to.be.revertedWithCustomError(policy, "OwnableUnauthorizedAccount") }) - it("should fail to set the target when the target address is zero", async () => { + it("reverts when zero address", async () => { const { policy, deployer } = await loadFixture(deployBasePolicyFixture) await expect(policy.connect(deployer).setTarget(ZeroAddress)).to.be.revertedWithCustomError( @@ -205,7 +205,7 @@ describe("Base", () => { ) }) - it("Should set the target contract address correctly", async () => { + it("sets target correctly", async () => { const { policy, target, BaseERC721PolicyFactory } = await loadFixture(deployBasePolicyFixture) const targetAddress = await target.getAddress() @@ -224,7 +224,7 @@ describe("Base", () => { expect(await policy.getTarget()).to.eq(targetAddress) }) - it("Should fail to set the target if already set", async () => { + it("reverts when already set", async () => { const { policy, target } = await loadFixture(deployBasePolicyFixture) const targetAddress = await target.getAddress() @@ -234,8 +234,8 @@ describe("Base", () => { }) }) - describe("enforce()", () => { - it("should throw when the callee is not the target", async () => { + describe("enforce", () => { + it("reverts when caller not target", async () => { const { policy, subject, target, subjectAddress } = await loadFixture(deployBasePolicyFixture) await policy.setTarget(await target.getAddress()) @@ -246,7 +246,7 @@ describe("Base", () => { ) }) - it("should throw when the evidence is not correct", async () => { + it("reverts when evidence invalid", async () => { const { iERC721Errors, policy, target, subjectAddress, invalidEncodedNFTId } = await loadFixture(deployBasePolicyFixture) @@ -257,7 +257,7 @@ describe("Base", () => { ).to.be.revertedWithCustomError(iERC721Errors, "ERC721NonexistentToken") }) - it("should throw when the check returns false", async () => { + it("reverts when check fails", async () => { const { policy, target, notOwnerAddress, validEncodedNFTId } = await loadFixture(deployBasePolicyFixture) @@ -268,7 +268,7 @@ describe("Base", () => { ).to.be.revertedWithCustomError(policy, "UnsuccessfulCheck") }) - it("should enforce", async () => { + it("enforces successfully", async () => { const { BaseERC721PolicyFactory, policy, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployBasePolicyFixture) const targetAddress = await target.getAddress() @@ -294,7 +294,7 @@ describe("Base", () => { expect(await policy.enforced(targetAddress, subjectAddress)).to.be.equal(true) }) - it("should prevent to enforce twice", async () => { + it("reverts when already enforced", async () => { const { policy, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployBasePolicyFixture) await policy.setTarget(await target.getAddress()) @@ -307,8 +307,8 @@ describe("Base", () => { }) }) - describe("_enforce()", () => { - it("should throw when the callee is not the target", async () => { + describe("internal enforce", () => { + it("reverts when caller not target", async () => { const { policyHarness, subject, target, subjectAddress } = await loadFixture(deployBasePolicyFixture) await policyHarness.setTarget(await target.getAddress()) @@ -318,7 +318,7 @@ describe("Base", () => { ).to.be.revertedWithCustomError(policyHarness, "TargetOnly") }) - it("should throw when the evidence is not correct", async () => { + it("reverts when evidence invalid", async () => { const { iERC721Errors, policyHarness, target, subjectAddress, invalidEncodedNFTId } = await loadFixture(deployBasePolicyFixture) @@ -329,7 +329,7 @@ describe("Base", () => { ).to.be.revertedWithCustomError(iERC721Errors, "ERC721NonexistentToken") }) - it("should throw when the check returns false", async () => { + it("reverts when check fails", async () => { const { policyHarness, target, notOwnerAddress, validEncodedNFTId } = await loadFixture(deployBasePolicyFixture) @@ -340,7 +340,7 @@ describe("Base", () => { ).to.be.revertedWithCustomError(policyHarness, "UnsuccessfulCheck") }) - it("should enforce", async () => { + it("enforces successfully", async () => { const { BaseERC721PolicyFactory, policyHarness, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployBasePolicyFixture) const targetAddress = await target.getAddress() @@ -366,7 +366,7 @@ describe("Base", () => { expect(await policyHarness.enforced(targetAddress, subjectAddress)).to.be.equal(true) }) - it("should prevent to enforce twice", async () => { + it("reverts when already enforced", async () => { const { policyHarness, target, subjectAddress, validEncodedNFTId } = await loadFixture(deployBasePolicyFixture) @@ -432,16 +432,16 @@ describe("Base", () => { } } - describe("constructor()", () => { - it("Should deploy the voting contract correctly", async () => { + describe("constructor", () => { + it("deploys correctly", async () => { const { voting } = await loadFixture(deployBaseVotingFixture) expect(voting).to.not.eq(undefined) }) }) - describe("register()", () => { - it("Should revert when the callee is not the target", async () => { + describe("register", () => { + it("reverts when caller not target", async () => { const { voting, policy, notOwner, validNFTId } = await loadFixture(deployBaseVotingFixture) await policy.setTarget(await notOwner.getAddress()) @@ -452,7 +452,7 @@ describe("Base", () => { ) }) - it("Should revert when the evidence is not correct", async () => { + it("reverts when evidence invalid", async () => { const { iERC721Errors, voting, policy, subject, invalidNFTId } = await loadFixture(deployBaseVotingFixture) @@ -464,7 +464,7 @@ describe("Base", () => { ) }) - it("should throw when the registration check returns false", async () => { + it("reverts when check fails", async () => { const { voting, policy, notOwner, validNFTId } = await loadFixture(deployBaseVotingFixture) await policy.setTarget(await voting.getAddress()) @@ -475,7 +475,7 @@ describe("Base", () => { ) }) - it("should register", async () => { + it("registers successfully", async () => { const { BaseVotingFactory, voting, policy, subject, validNFTId, subjectAddress } = await loadFixture(deployBaseVotingFixture) const targetAddress = await voting.getAddress() @@ -500,7 +500,7 @@ describe("Base", () => { expect(await voting.voteCounts(1)).to.be.equal(0) }) - it("should prevent to register twice", async () => { + it("reverts when already registered", async () => { const { voting, policy, subject, validNFTId } = await loadFixture(deployBaseVotingFixture) const targetAddress = await voting.getAddress() @@ -515,8 +515,8 @@ describe("Base", () => { }) }) - describe("vote()", () => { - it("Should revert when the callee is not registered", async () => { + describe("vote", () => { + it("reverts when not registered", async () => { const { voting, policy, subject } = await loadFixture(deployBaseVotingFixture) await policy.setTarget(await voting.getAddress()) @@ -524,7 +524,7 @@ describe("Base", () => { await expect(voting.connect(subject).vote(0)).to.be.revertedWithCustomError(voting, "NotRegistered") }) - it("Should revert when the option is not correct", async () => { + it("reverts when option invalid", async () => { const { voting, policy, subject, validNFTId } = await loadFixture(deployBaseVotingFixture) await policy.setTarget(await voting.getAddress()) @@ -533,7 +533,7 @@ describe("Base", () => { await expect(voting.connect(subject).vote(3)).to.be.revertedWithCustomError(voting, "InvalidOption") }) - it("should vote", async () => { + it("votes successfully", async () => { const { BaseVotingFactory, voting, policy, subject, subjectAddress, validNFTId } = await loadFixture(deployBaseVotingFixture) const option = 0 @@ -560,7 +560,7 @@ describe("Base", () => { expect(await voting.voteCounts(1)).to.be.equal(0) }) - it("should prevent to vote twice", async () => { + it("reverts when already voted", async () => { const { voting, policy, subject, validNFTId } = await loadFixture(deployBaseVotingFixture) const targetAddress = await voting.getAddress() @@ -573,8 +573,8 @@ describe("Base", () => { }) }) - describe("e2e", () => { - it("should submit a vote for each subject", async () => { + describe("end to end", () => { + it("completes full voting flow", async () => { const [deployer]: Signer[] = await ethers.getSigners() const NFTFactory: NFT__factory = await ethers.getContractFactory("NFT")