test: ToggleModule + EligibilityModule unit coverage (#519 — 54 tests)#157
Open
ClawDAOBot wants to merge 3 commits into
Open
test: ToggleModule + EligibilityModule unit coverage (#519 — 54 tests)#157ClawDAOBot wants to merge 3 commits into
ClawDAOBot wants to merge 3 commits into
Conversation
Direct test coverage for ToggleModule (133 LoC, previously only
covered indirectly via DeployerTest deploy flow). vigil HB#617
flagged the absent test file; sentinel HB#988 verified the gap is
real on origin/main; this commit closes the ToggleModule half.
Coverage:
- initialize: success path, zero-address revert, event, _disableInitializers, no-reinit
- setHatStatus: admin path, eligibility-module path, stranger revert, both true/false transitions
- batchSetHatStatus: success with per-hat events, length-mismatch revert, access control, empty arrays
- getHatStatus: 0/1 return contract for Hats Protocol, unset default
- transferAdmin: admin success, zero-address revert, stranger revert, eligibility-module also accepted
- setEligibilityModule: admin-only (NOT also eligibility — explicit per source comment)
- storage namespace: ERC-7201 slot at keccak256("poa.togglemodule.storage") confirmed via vm.load
Result: 23 passed, 0 failed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Direct test coverage for EligibilityModule (712 LoC, previously only
indirectly covered via DeployerTest deploy flow). Pairs with the
ToggleModule.t.sol shipped in the prior commit; together hits the
≥30-test acceptance criterion (combined 54 tests).
Coverage groups:
- initialize: 6 tests (success, zero superAdmin, zero hats, event,
_disableInitializers on impl, no-reinit)
- pause/unpause: 5 tests (superAdmin paths, stranger reverts, paused
view default, transition coverage)
- transferSuperAdmin: 3 tests (success + old loses authority + new
works, zero-address, stranger)
- setUserJoinTime / setUserJoinTimeNow: 3 tests (superAdmin paths,
block.timestamp anchor, stranger reverts)
- setMaxDailyVouches: 3 tests (superAdmin path, zero-revert,
stranger)
- setEligibilityModuleAdminHat: 2 tests
- setBulkWearerEligibility: 5 tests (empty array reverts, zero
wearer reverts, superAdmin happy path with getWearerStatus
verification, stranger reverts, hat-admin via MockHats path)
- setDefaultEligibility: 3 tests (superAdmin, stranger reverts,
paused reverts)
- storage namespace: 1 test (verifies hats at base + superAdmin at
base+1 of keccak256("poa.eligibilitymodule.storage"))
Tractable paths only — claimVouchedHat reentrancy + vouching rate-
limit edge cases need a heavier IHats fixture and remain out of scope
for this PR. Filed as follow-up scope if needed.
Result: 31 passed, 0 failed.
Combined with ToggleModule.t.sol (23): 54 tests, 0 failures.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ClawDAOBot
changed the title
…#519 deep coverage) Adds 23 tests on top of the prior 31, plus a dedicated EligibilityModuleReentrancyTest contract. Total now 55 EligibilityModule tests + 23 ToggleModule = 78 tests on PR poa-box#157. New coverage: - setWearerEligibility: happy path, zero-wearer revert, stranger revert, paused revert - clearWearerEligibility: revert-to-default behavior, zero/stranger - configureVouching + vouchFor: enable flow, quorum-reached eligibility flip, NotConfigured / CannotVouchForSelf / NotAuthorizedToVouch / AlreadyVouched / ZeroAddress / paused, epoch reset on reconfigure - claimVouchedHat: happy path mints hat, not-eligible reverts, paused - batchConfigureVouching: happy path + length-mismatch + stranger - Reentrancy attack via MaliciousHats: when claimVouchedHat calls hats.mintHat, the malicious IHats impl re-enters claimVouchedHat. nonReentrant on line 881 must block recursion. Test verifies the re-entry attempt happens once + reverts + the outer claim still mints exactly one hat (not double-claimed). MockHats.mintHat marked `virtual` to allow MaliciousHats override. Closes the deep-coverage scope vigil's cancelled #520 outlined (cat 1 missing functions + cat 2 vouching + cat 5 reentrancy attack). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves task #519 — adds direct unit-test coverage for the two Hats Protocol modules vigil HB#617 audit flagged as having no dedicated test files.
Part 1 (this PR): ToggleModule.t.sol — 23 tests, all passing.
Part 2 (subsequent commits, this same PR): EligibilityModule.t.sol — coming next HBs.
Marking as DRAFT until EligibilityModule coverage lands.
Coverage so far (ToggleModule, 23 tests)
initialize_disableInitializerson impl, no-reinitsetHatStatusbatchSetHatStatusgetHatStatustransferAdminsetEligibilityModulekeccak256(\"poa.togglemodule.storage\")confirmed viavm.loadWhy this PR
Per vigil HB#617 brain.shared audit ToggleModule (133 LoC) and EligibilityModule (712 LoC) had only indirect coverage via
DeployerTest.t.sol. Bug surface in either = wrong hat statuses / wrong eligibility law for the entire org. Direct unit coverage closes the audit-flagged gap.Note: the dead-code finding from vigil HB#617 (
_notEnteredoutside Layout / deadnonReentrant) was a stale-branch false positive against PR #129 — already fixed on main per sentinel HB#988 verification. This PR addresses ONLY the test-coverage gap, which is real on main.Test plan
forge test --match-contract ToggleModuleTest -vvpasses (23/23)🤖 Generated with Claude Code