feat: Implement 11 security fixes and add comprehensive tests

src/DirectDemocracyVoting.sol

@@ -46,6 +46,7 @@ contract DirectDemocracyVoting is Initializable {
         uint256[] pollHatIds; // array of specific hat IDs for this poll
         bool restricted; // if true only allowedHats can vote
         mapping(uint256 => bool) pollHatAllowed; // O(1) lookup for poll hat permission
+        bool executed; // finalization guard
     }
 
     /* ─────────── ERC-7201 Storage ─────────── */
@@ -408,9 +409,13 @@ contract DirectDemocracyVoting is Initializable {
         whenNotPaused
         returns (uint256 winner, bool valid)
     {
-        (winner, valid) = _calcWinner(id);
         Layout storage l = _layout();
-        IExecutor.Call[] storage batch = l._proposals[id].batches[winner];
+        Proposal storage prop = l._proposals[id];
+        if (prop.executed) revert VotingErrors.AlreadyExecuted();
+        prop.executed = true;
+
+        (winner, valid) = _calcWinner(id);
+        IExecutor.Call[] storage batch = prop.batches[winner];
 
         if (valid && batch.length > 0) {
             uint256 len = batch.length;

src/EducationHub.sol

@@ -198,7 +198,7 @@ contract EducationHub is Initializable, ContextUpgradeable, ReentrancyGuardUpgra
         }
 
         l._modules[id] =
-            Module({answerHash: keccak256(abi.encodePacked(correctAnswer)), payout: uint128(payout), exists: true});
+            Module({answerHash: keccak256(abi.encodePacked(id, correctAnswer)), payout: uint128(payout), exists: true});
 
         emit ModuleCreated(id, title, contentHash, payout);
     }
@@ -229,7 +229,7 @@ contract EducationHub is Initializable, ContextUpgradeable, ReentrancyGuardUpgra
         Layout storage l = _layout();
         Module storage m = _module(l, id);
         if (_isCompleted(l, _msgSender(), id)) revert AlreadyCompleted();
-        if (keccak256(abi.encodePacked(answer)) != m.answerHash) revert InvalidAnswer();
+        if (keccak256(abi.encodePacked(uint48(id), answer)) != m.answerHash) revert InvalidAnswer();
 
         l.token.mint(_msgSender(), m.payout);
         _setCompleted(l, _msgSender(), id);

src/EligibilityModule.sol

@@ -111,7 +111,7 @@ contract EligibilityModule is Initializable, IHatsEligibility {
     uint256 private _notEntered = 1;
 
     modifier nonReentrant() {
-        require(_notEntered == 1, "ReentrancyGuard: reentrant call");
+        require(_notEntered != 2, "ReentrancyGuard: reentrant call");
         _notEntered = 2;
         _;
         _notEntered = 1;
@@ -197,6 +197,8 @@ contract EligibilityModule is Initializable, IHatsEligibility {
     function initialize(address _superAdmin, address _hats, address _toggleModule) external initializer {
         if (_superAdmin == address(0) || _hats == address(0)) revert ZeroAddress();
 
+        _notEntered = 1;
+
         Layout storage l = _layout();
         l.superAdmin = _superAdmin;
         l.hats = IHats(_hats);
@@ -768,9 +770,8 @@ contract EligibilityModule is Initializable, IHatsEligibility {
         uint32 newCount = l.currentVouchCount[hatId][wearer] - 1;
         l.currentVouchCount[hatId][wearer] = newCount;
 
-        uint256 currentDay = block.timestamp / SECONDS_PER_DAY;
-        uint32 dailyCount = l.dailyVouchCount[msg.sender][currentDay] - 1;
-        l.dailyVouchCount[msg.sender][currentDay] = dailyCount;
+        // Note: dailyVouchCount is NOT decremented on revocation.
+        // It's a rate limiter only — revoking doesn't give back vouch slots.
 
         emit VouchRevoked(msg.sender, wearer, hatId, newCount);
 
@@ -795,7 +796,7 @@ contract EligibilityModule is Initializable, IHatsEligibility {
      *      The EligibilityModule contract mints the hat using its ELIGIBILITY_ADMIN permissions.
      * @param hatId The ID of the hat to claim
      */
-    function claimVouchedHat(uint256 hatId) external whenNotPaused {
+    function claimVouchedHat(uint256 hatId) external whenNotPaused nonReentrant {
         Layout storage l = _layout();
 
         // Check if caller is eligible to claim this hat
@@ -805,13 +806,13 @@ contract EligibilityModule is Initializable, IHatsEligibility {
         // Check if already wearing the hat
         require(!l.hats.isWearerOfHat(msg.sender, hatId), "Already wearing hat");
 
+        // State change BEFORE external call (CEI pattern)
+        delete l.roleApplications[hatId][msg.sender];
+
         // Mint the hat to the caller using EligibilityModule's admin powers
         bool success = l.hats.mintHat(hatId, msg.sender);
         require(success, "Hat minting failed");
 
-        // Clean up any pending role application
-        delete l.roleApplications[hatId][msg.sender];
-
         emit HatClaimed(msg.sender, hatId);
     }
 

src/Executor.sol

@@ -78,10 +78,7 @@ contract Executor is Initializable, OwnableUpgradeable, PausableUpgradeable, Ree
     function setCaller(address newCaller) external {
         if (newCaller == address(0)) revert ZeroAddress();
         Layout storage l = _layout();
-        if (l.allowedCaller != address(0)) {
-            // After first set, only current caller or owner can change
-            if (msg.sender != l.allowedCaller && msg.sender != owner()) revert UnauthorizedCaller();
-        }
+        if (msg.sender != l.allowedCaller && msg.sender != owner()) revert UnauthorizedCaller();
         l.allowedCaller = newCaller;
         emit CallerSet(newCaller);
     }

src/HybridVoting.sol

@@ -52,6 +52,7 @@ contract HybridVoting is Initializable {
         bool restricted; // if true only pollHatIds can vote
         mapping(uint256 => bool) pollHatAllowed; // O(1) lookup for poll hat permission
         ClassConfig[] classesSnapshot; // Snapshot the class config to freeze semantics for this proposal
+        bool executed; // finalization guard
     }
 
     /* ─────── ERC-7201 Storage ─────── */

src/OrgDeployer.sol

@@ -174,14 +174,12 @@ contract OrgDeployer is Initializable {
 
     /**
      * @notice Set the universal passkey factory address
-     * @dev Callable by PoaManager, or anyone for one-time initial setup (when factory is not yet set)
+     * @dev Only callable by PoaManager
      */
     function setUniversalPasskeyFactory(address _universalFactory) external {
         Layout storage l = _layout();
-        // Allow one-time setup by anyone (when factory is not yet set), or require PoaManager for updates
-        if (l.universalPasskeyFactory != address(0) && msg.sender != l.poaManager) {
-            revert InvalidAddress();
-        }
+        if (msg.sender != l.poaManager) revert InvalidAddress();
+        if (_universalFactory == address(0)) revert InvalidAddress();
         l.universalPasskeyFactory = _universalFactory;
     }
 

src/PasskeyAccount.sol

@@ -236,6 +236,7 @@ contract PasskeyAccount is Initializable, IAccount, IPasskeyAccount {
 
     /// @inheritdoc IPasskeyAccount
     function addCredential(bytes32 credentialId, bytes32 pubKeyX, bytes32 pubKeyY) external override onlySelf {
+        if (pubKeyX == bytes32(0) || pubKeyY == bytes32(0)) revert InvalidSignature();
         Layout storage l = _layout();
 
         // Check if credential already exists
@@ -319,6 +320,7 @@ contract PasskeyAccount is Initializable, IAccount, IPasskeyAccount {
 
     /// @inheritdoc IPasskeyAccount
     function initiateRecovery(bytes32 credentialId, bytes32 pubKeyX, bytes32 pubKeyY) external override onlyGuardian {
+        if (pubKeyX == bytes32(0) || pubKeyY == bytes32(0)) revert InvalidSignature();
         Layout storage l = _layout();
 
         // Generate recovery ID
@@ -361,6 +363,12 @@ contract PasskeyAccount is Initializable, IAccount, IPasskeyAccount {
             revert RecoveryDelayNotPassed();
         }
 
+        // Check max credentials limit before adding
+        uint8 maxCreds = _getMaxCredentials();
+        if (l.credentialIds.length >= maxCreds) {
+            revert MaxCredentialsReached();
+        }
+
         // Add the new credential
         bytes32 credentialId = request.credentialId;
 

src/PaymasterHub.sol

@@ -126,6 +126,7 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG
         address entryPoint;
         address hats;
         address poaManager;
+        address orgRegistrar; // authorized contract that can register orgs (e.g. OrgDeployer)
     }
 
     // keccak256(abi.encode(uint256(keccak256("poa.paymasterhub.main")) - 1))
@@ -335,6 +336,8 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG
     function registerOrgWithVoucher(bytes32 orgId, uint256 adminHatId, uint256 operatorHatId, uint256 voucherHatId)
         public
     {
+        MainStorage storage main = _getMainStorage();
+        if (msg.sender != main.poaManager && msg.sender != main.orgRegistrar) revert NotPoaManager();
         if (adminHatId == 0) revert ZeroAddress();
 
         mapping(bytes32 => OrgConfig) storage orgs = _getOrgsStorage();
@@ -611,18 +614,17 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG
         mapping(bytes32 => OrgFinancials) storage financials = _getFinancialsStorage();
         OrgFinancials storage org = financials[orgId];
 
-        // Calculate total available funds
-        uint256 totalAvailable = uint256(org.deposited) - uint256(org.spent);
+        uint256 depositAvailable =
+            uint256(org.deposited) > uint256(org.spent) ? uint256(org.deposited) - uint256(org.spent) : 0;
 
-        // Check if org has enough in deposits to cover this
-        // Note: solidarity is checked separately in _checkSolidarityAccess
-        if (org.spent + maxCost > org.deposited) {
-            // Will need to use solidarity - that's checked elsewhere
-            // Here we just make sure they haven't overdrawn
-            if (totalAvailable == 0) {
-                revert InsufficientOrgBalance();
-            }
+        // If deposits alone cover the cost, no solidarity needed
+        if (depositAvailable >= maxCost) return;
+
+        // Deposits are fully exhausted — revert early before solidarity check
+        if (depositAvailable == 0) {
+            revert InsufficientOrgBalance();
         }
+        // Partial coverage: solidarity will cover the rest (validated by _checkSolidarityAccess)
     }
 
     /**
@@ -1033,6 +1035,16 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG
         emit GracePeriodConfigUpdated(_initialGraceDays, _maxSpendDuringGrace, _minDepositRequired);
     }
 
+    /**
+     * @notice Set the authorized org registrar (e.g. OrgDeployer)
+     * @dev Only PoaManager can set the registrar
+     * @param registrar Address authorized to call registerOrg
+     */
+    function setOrgRegistrar(address registrar) external {
+        if (msg.sender != _getMainStorage().poaManager) revert NotPoaManager();
+        _getMainStorage().orgRegistrar = registrar;
+    }
+
     /**
      * @notice Ban or unban an org from accessing solidarity fund
      * @dev Only PoaManager can ban orgs for malicious behavior
@@ -1665,13 +1677,10 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG
         }
 
         if (tip > 0) {
-            // Update total paid
-            bounty.totalPaid += uint144(tip);
-
-            // Attempt payment with gas limit
             (bool success,) = bundlerOrigin.call{value: tip, gas: 30000}("");
 
             if (success) {
+                bounty.totalPaid += uint144(tip);
                 emit BountyPaid(userOpHash, bundlerOrigin, tip);
             } else {
                 emit BountyPayFailed(userOpHash, bundlerOrigin, tip);

src/TaskManager.sol

@@ -41,6 +41,7 @@ contract TaskManager is Initializable, ContextUpgradeable {
     error RequiresApplication();
     error NoApplicationRequired();
     error InvalidIndex();
+    error SelfReviewNotAllowed();
 
     /*──────── Constants ─────*/
     bytes4 public constant MODULE_ID = 0x54534b32; // "TSK2"
@@ -499,10 +500,19 @@ contract TaskManager is Initializable, ContextUpgradeable {
 
     function completeTask(uint256 id) external {
         Layout storage l = _layout();
-        _checkPerm(l._tasks[id].projectId, TaskPerm.REVIEW);
+        bytes32 pid = l._tasks[id].projectId;
+        _checkPerm(pid, TaskPerm.REVIEW);
         Task storage t = _task(l, id);
         if (t.status != Status.SUBMITTED) revert BadStatus();
 
+        // Self-review: if caller is the claimer, require SELF_REVIEW permission or PM/executor
+        address sender = _msgSender();
+        if (t.claimer == sender && !_isPM(pid, sender)) {
+            if (!TaskPerm.has(_permMask(sender, pid), TaskPerm.SELF_REVIEW)) {
+                revert SelfReviewNotAllowed();
+            }
+        }
+
         t.status = Status.COMPLETED;
         l.token.mint(t.claimer, uint256(t.payout));
 

src/libs/HybridVotingCore.sol

@@ -137,6 +137,8 @@ library HybridVotingCore {
     function announceWinner(uint256 id) external returns (uint256 winner, bool valid) {
         HybridVoting.Layout storage l = _layout();
         HybridVoting.Proposal storage p = l._proposals[id];
+        if (p.executed) revert VotingErrors.AlreadyExecuted();
+        p.executed = true;
 
         // Check if any votes were cast
         bool hasVotes = false;
@@ -191,14 +193,14 @@ library HybridVotingCore {
         );
 
         IExecutor.Call[] storage batch = p.batches[winner];
-        bool executed = false;
+        bool didExecute = false;
         if (valid && batch.length > 0) {
             // No target validation needed - Executor has onlyExecutor permission on all org contracts
             // and handles the actual calls. HybridVoting just passes the batch through.
             l.executor.execute(id, batch);
-            executed = true;
+            didExecute = true;
             emit ProposalExecuted(id, winner, batch.length);
         }
-        emit Winner(id, winner, valid, executed, uint64(block.timestamp));
+        emit Winner(id, winner, valid, didExecute, uint64(block.timestamp));
     }
 }

src/libs/TaskPerm.sol

@@ -10,6 +10,7 @@ library TaskPerm {
     uint8 internal constant CLAIM = 1 << 1;
     uint8 internal constant REVIEW = 1 << 2;
     uint8 internal constant ASSIGN = 1 << 3;
+    uint8 internal constant SELF_REVIEW = 1 << 4;
 
     function has(uint8 mask, uint8 flag) internal pure returns (bool) {
         return mask & flag != 0;

src/libs/VotingErrors.sol

@@ -29,4 +29,5 @@ library VotingErrors {
     error InvalidSliceSum();
     error TooManyClasses();
     error InvalidStrategy();
+    error AlreadyExecuted();
 }

test/DeployerTest.t.sol

@@ -726,6 +726,12 @@ contract DeployerTest is Test, IEligibilityModuleEvents {
         );
         deployer = OrgDeployer(address(new BeaconProxy(deployerBeacon, deployerInit)));
 
+        // Authorize OrgDeployer to register orgs on PaymasterHub
+        vm.stopPrank();
+        vm.prank(address(poaManager));
+        paymasterHub.setOrgRegistrar(address(deployer));
+        vm.startPrank(poaAdmin);
+
         // Debug to verify Deployer initialization
         console.log("deployer address:", address(deployer));
 
@@ -3980,4 +3986,35 @@ contract DeployerTest is Test, IEligibilityModuleEvents {
         address beacon = _getBeaconForType(ModuleTypes.TOGGLE_MODULE_ID);
         assertEq(SwitchableBeacon(beacon).owner(), setup.exec, "Toggle beacon should be owned by executor");
     }
+
+    function testVouchRevocationCrossDay() public {
+        TestOrgSetup memory setup = _createTestOrg("Cross-Day Revoke DAO");
+        address candidate = address(0x500);
+
+        _setupUserForVouching(setup.eligibilityModule, setup.exec, voter1);
+        _setupUserForVouching(setup.eligibilityModule, setup.exec, voter2);
+        _setupUserForVouching(setup.eligibilityModule, setup.exec, candidate);
+
+        _configureVouching(
+            setup.eligibilityModule, setup.exec, setup.defaultRoleHat, 2, setup.memberRoleHat, false, true
+        );
+
+        _mintHat(setup.exec, setup.memberRoleHat, voter1);
+        _mintHat(setup.exec, setup.memberRoleHat, voter2);
+
+        // Vouch on day 1
+        _vouchFor(voter1, setup.eligibilityModule, candidate, setup.defaultRoleHat);
+        _vouchFor(voter2, setup.eligibilityModule, candidate, setup.defaultRoleHat);
+
+        // Warp to a different day (previously caused underflow in dailyVouchCount decrement)
+        vm.warp(block.timestamp + 2 days);
+
+        // Revoke on day 3 — should succeed without underflow
+        _revokeVouch(voter1, setup.eligibilityModule, candidate, setup.defaultRoleHat);
+
+        // Verify the revocation worked correctly
+        _assertVouchStatus(
+            setup.eligibilityModule, candidate, setup.defaultRoleHat, 1, false, "After cross-day revocation"
+        );
+    }
 }

test/DirectDemocracyVoting.t.sol

@@ -612,4 +612,32 @@ contract DDVotingTest is Test {
         assertEq(hats.balanceOf(bob, executiveHatId), 0, "Bob should not have executive hat");
         assertEq(hats.balanceOf(bob, managerHatId), 0, "Bob should not have manager hat");
     }
+
+    /*////////////////////////////////////////////////////////////
+                    ANNOUNCE WINNER REPLAY PROTECTION
+    ////////////////////////////////////////////////////////////*/
+
+    function testAnnounceWinnerDoubleCallReverts() public {
+        IExecutor.Call[][] memory b = new IExecutor.Call[][](2);
+        b[0] = new IExecutor.Call[](0);
+        b[1] = new IExecutor.Call[](0);
+        vm.prank(creator);
+        dd.createProposal(bytes("Replay Test"), bytes32(0), 10, 2, b, new uint256[](0));
+
+        uint8[] memory idx = new uint8[](1);
+        idx[0] = 0;
+        uint8[] memory w = new uint8[](1);
+        w[0] = 100;
+        vm.prank(voter);
+        dd.vote(0, idx, w);
+
+        vm.warp(block.timestamp + 11 minutes);
+
+        // First call succeeds
+        dd.announceWinner(0);
+
+        // Second call reverts
+        vm.expectRevert(VotingErrors.AlreadyExecuted.selector);
+        dd.announceWinner(0);
+    }
 }