diff --git a/docs/HYBRID_VOTING.md b/docs/HYBRID_VOTING.md index 0ba0052..584ed3a 100644 --- a/docs/HYBRID_VOTING.md +++ b/docs/HYBRID_VOTING.md @@ -471,6 +471,18 @@ Hats enables dynamic, attestation-based roles without redeploying contracts. | Whale dominance | Quadratic voting dampens large holders | | Empty proposal spam | Creator hat requirement | | Mid-vote manipulation | Class configuration snapshots | +| Zero-weight Sybils | A voter with no power in any class is rejected (`Unauthorized`) — cannot pad turnout/quorum | + +### ERC20_BAL weight is read **live** — safe-configuration requirements + +⚠️ **Important nuance.** "Snapshot Isolation" above freezes the *class configuration* (strategy, slice, asset, hat gating) at proposal creation — it does **not** snapshot token *balances*. An `ERC20_BAL` class reads `IERC20(asset).balanceOf(voter)` at the moment `vote()` is called, not a `getPastVotes` checkpoint. This is intentional and safe **only when the org is configured as follows**: + +1. **Use the soulbound ParticipationToken as the `ERC20_BAL` asset.** PT's `transfer`/`transferFrom`/`approve`/`delegate` all revert, so the classic *transfer-and-revote* / flash-loan inflation is structurally impossible — the same tokens cannot be moved between addresses to be counted twice within one proposal. +2. **Keep PT mint authority narrow.** Because weight is live, anyone who can mint PT mid-proposal can raise their own vote weight. Mint is gated to the TaskManager / EducationHub / approvers, and an approver **cannot self-approve** their own token request. Do **not** grant broad project `SELF_REVIEW` (which lets a contributor self-complete tasks and mint themselves PT) to members you would not trust to inflate a vote. Use designated project leads and fixed project budgets. + +🚫 **Do not** configure an `ERC20_BAL` class over a freely **transferable** ERC20 unless that token is checkpoint-based and you have added an explicit `getPastVotes` snapshot — with a plain transferable token, live `balanceOf` allows transfer-and-revote across colluding hat-wearers. + +These boundaries are proven by `test/HybridVotingSafeConfig.t.sol`: soulbound transfers revert, unprivileged actors cannot self-mint, a decided outcome cannot be flipped without mint authority, and a 128k-call fuzzing invariant shows unprivileged voting/transfer/mint activity never changes PT supply or voter balances. The same file's `test_Boundary_MintAuthorityIsTheLever` demonstrates, executably, the inflation that an *unsafe* config (broad mint authority) would expose. --- diff --git a/script/deploy/DeployInfrastructure.s.sol b/script/deploy/DeployInfrastructure.s.sol index 0fc9624..2f8f1c5 100644 --- a/script/deploy/DeployInfrastructure.s.sol +++ b/script/deploy/DeployInfrastructure.s.sol @@ -229,9 +229,10 @@ contract DeployInfrastructure is Script { .adminCall( paymasterHub, abi.encodeWithSignature( - "setOnboardingConfig(uint128,uint128,bool,address)", + "setOnboardingConfig(uint128,uint128,uint8,bool,address)", uint128(0.01 ether), uint128(1000), + uint8(3), true, globalAccountRegistry ) diff --git a/script/deploy/MainDeploy.s.sol b/script/deploy/MainDeploy.s.sol index 87fe93a..156175c 100644 --- a/script/deploy/MainDeploy.s.sol +++ b/script/deploy/MainDeploy.s.sol @@ -252,9 +252,10 @@ contract DeployHomeChain is DeployHelper { .adminCall( infra.paymasterHub, abi.encodeWithSignature( - "setOnboardingConfig(uint128,uint128,bool,address)", + "setOnboardingConfig(uint128,uint128,uint8,bool,address)", uint128(0.01 ether), uint128(1000), + uint8(3), true, infra.globalAccountRegistry ) @@ -667,9 +668,10 @@ contract DeploySatellite is DeployHelper { pm.adminCall( infra.paymasterHub, abi.encodeWithSignature( - "setOnboardingConfig(uint128,uint128,bool,address)", + "setOnboardingConfig(uint128,uint128,uint8,bool,address)", uint128(0.01 ether), uint128(1000), + uint8(3), true, infra.globalAccountRegistry ) diff --git a/script/upgrades/UpgradeEligibilitySuperAdminLockdown.s.sol b/script/upgrades/UpgradeEligibilitySuperAdminLockdown.s.sol index 297a326..14521f6 100644 --- a/script/upgrades/UpgradeEligibilitySuperAdminLockdown.s.sol +++ b/script/upgrades/UpgradeEligibilitySuperAdminLockdown.s.sol @@ -316,9 +316,7 @@ contract DryRun_GnosisUpgrade is Script { vm.prank(KUBI_EXECUTOR); (bool okExecOn,) = KUBI_ELIG_MODULE.call( - abi.encodeWithSignature( - "setWearerEligibility(address,uint256,bool,bool)", probe, MEMBER_HAT, true, true - ) + abi.encodeWithSignature("setWearerEligibility(address,uint256,bool,bool)", probe, MEMBER_HAT, true, true) ); require(okExecOn, "DryRun: superAdmin setWearerEligibility(true,true) reverted"); { @@ -328,9 +326,7 @@ contract DryRun_GnosisUpgrade is Script { vm.prank(KUBI_EXECUTOR); (bool okExecOff,) = KUBI_ELIG_MODULE.call( - abi.encodeWithSignature( - "setWearerEligibility(address,uint256,bool,bool)", probe, MEMBER_HAT, false, false - ) + abi.encodeWithSignature("setWearerEligibility(address,uint256,bool,bool)", probe, MEMBER_HAT, false, false) ); require(okExecOff, "DryRun: superAdmin setWearerEligibility(false,false) reverted"); { diff --git a/script/upgrades/UpgradePaymasterOnboardingCap.s.sol b/script/upgrades/UpgradePaymasterOnboardingCap.s.sol new file mode 100644 index 0000000..76b391c --- /dev/null +++ b/script/upgrades/UpgradePaymasterOnboardingCap.s.sol @@ -0,0 +1,223 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity ^0.8.20; + +import "forge-std/Script.sol"; +import "forge-std/console.sol"; +import {PaymasterHub} from "../../src/PaymasterHub.sol"; +import {PoaManagerHub} from "../../src/crosschain/PoaManagerHub.sol"; +import {PoaManager} from "../../src/PoaManager.sol"; +import {DeterministicDeployer} from "../../src/crosschain/DeterministicDeployer.sol"; + +// ───────────────────────────────────────────────────────────────────────────── +// UpgradePaymasterOnboardingCap (Audit H4) +// +// Adds a per-account lifetime cap on solidarity-funded onboarding sponsorship to +// PaymasterHub (new `OnboardingConfig.maxOnboardingsPerAccount`, a new +// `poa.paymasterhub.onboarding.counts` storage mapping, enforcement in +// `_validateOnboardingEligibility`, and the new `setOnboardingConfig` arg). +// +// Storage safety: the new struct field is appended (packs into the slot holding +// `accountRegistry`) and the counts mapping lives at a fresh ERC-7201 slot, so the +// upgrade preserves all existing onboarding state. `maxOnboardingsPerAccount == 0` +// means UNLIMITED, so an already-deployed hub (where the appended field reads 0 +// post-upgrade) keeps sponsoring onboarding until an admin sets a cap — Step3/Step4 +// set the cap to activate the protection. +// +// Validated: Sim_GnosisUpgrade PASSES under FOUNDRY_PROFILE=production via `forge script` +// (scoped compile of this script + its src deps) against a live Gnosis fork — i.e. a real +// production-profile sim, broadcast-representative bytecode. Run it with: +// FOUNDRY_PROFILE=production forge script \ +// script/upgrades/UpgradePaymasterOnboardingCap.s.sol:Sim_GnosisUpgrade --fork-url gnosis +// Note: a full-project `FOUNDRY_PROFILE=production forge build` currently fails with a +// Stack-too-deep in an unrelated NON-deployable test/script file (all of src/ compiles +// clean under production, and CI gates on the default profile), so it does not affect this +// scoped broadcast — but if you want a clean full production build, that file needs a fix. +// +// Version: v18 (probed free on Gnosis + Arbitrum, both registry and CREATE2, 2026-06-09). +// ───────────────────────────────────────────────────────────────────────────── + +// Shared constants (same addresses as UpgradePaymasterGraceFix) +address constant DD = 0x4aC8B5ebEb9D8C3dE3180ddF381D552d59e8835a; +address constant HUB = 0xB72840B343654eAfb2CFf7acC4Fc6b59E6c3CC71; // PoaManagerHub (Arbitrum) +address constant ARB_PAYMASTER = 0xD6659bCaFAdCB9CC2F57B7aE923c7F1Ca4438a11; +address constant GNOSIS_PAYMASTER = 0xdEf1038C297493c0b5f82F0CDB49e929B53B4108; +address constant GNOSIS_POA_MANAGER = 0x794fD39e75140ee1545B1B022E5486B7c863789b; +uint256 constant HYPERLANE_FEE = 0.005 ether; +string constant VERSION = "v18"; +uint8 constant MAX_ONBOARDINGS_PER_ACCOUNT = 3; // register + profile + 1 retry + +/// @title Step1_DeployImplOnGnosis — deploy PaymasterHub v18 impl on Gnosis via DD. +/// Usage: FOUNDRY_PROFILE=production forge script .../UpgradePaymasterOnboardingCap.s.sol:Step1_DeployImplOnGnosis --rpc-url gnosis --broadcast --slow --optimizer-runs 200 +contract Step1_DeployImplOnGnosis is Script { + function run() public { + uint256 deployerKey = vm.envOr("PRIVATE_KEY", vm.envUint("DEPLOYER_PRIVATE_KEY")); + DeterministicDeployer dd = DeterministicDeployer(DD); + + bytes32 salt = dd.computeSalt("PaymasterHub", VERSION); + address predicted = dd.computeAddress(salt); + console.log("Predicted PaymasterHub v18 impl:", predicted); + if (predicted.code.length > 0) { + console.log("Already deployed. Skipping."); + return; + } + vm.startBroadcast(deployerKey); + address deployed = dd.deploy(salt, type(PaymasterHub).creationCode); + vm.stopBroadcast(); + require(deployed == predicted, "Address mismatch"); + console.log("Deployed:", deployed); + console.log("Next: Step2_UpgradeFromArbitrum on Arbitrum"); + } +} + +/// @title Step2_UpgradeFromArbitrum — deploy on Arbitrum via DD + upgrade beacon cross-chain. +/// Usage: FOUNDRY_PROFILE=production forge script .../:Step2_UpgradeFromArbitrum --rpc-url arbitrum --broadcast --slow --optimizer-runs 200 +contract Step2_UpgradeFromArbitrum is Script { + function run() public { + uint256 deployerKey = vm.envOr("PRIVATE_KEY", vm.envUint("DEPLOYER_PRIVATE_KEY")); + address deployer = vm.addr(deployerKey); + PoaManagerHub hub = PoaManagerHub(payable(HUB)); + DeterministicDeployer dd = DeterministicDeployer(DD); + + require(hub.owner() == deployer, "Deployer must own Hub"); + require(!hub.paused(), "Hub is paused"); + + bytes32 salt = dd.computeSalt("PaymasterHub", VERSION); + address predicted = dd.computeAddress(salt); + + vm.startBroadcast(deployerKey); + if (predicted.code.length == 0) { + dd.deploy(salt, type(PaymasterHub).creationCode); + console.log("Deployed v18 on Arbitrum"); + } + hub.upgradeBeaconCrossChain{value: HYPERLANE_FEE}("PaymasterHub", predicted, VERSION); + console.log("Beacon upgraded cross-chain to v18"); + vm.stopBroadcast(); + console.log("Wait ~5 min for Hyperlane relay, then run Step3 (Gnosis) and Step4 (Arbitrum) to set the cap."); + } +} + +/// @title Step3_SetCapGnosis — activate the per-account cap on the Gnosis paymaster (preserves other config). +/// Usage: FOUNDRY_PROFILE=production forge script .../:Step3_SetCapGnosis --rpc-url gnosis --broadcast --slow --optimizer-runs 200 +contract Step3_SetCapGnosis is Script { + function run() public { + uint256 deployerKey = vm.envOr("PRIVATE_KEY", vm.envUint("DEPLOYER_PRIVATE_KEY")); + PaymasterHub pm = PaymasterHub(payable(GNOSIS_PAYMASTER)); + PaymasterHub.OnboardingConfig memory c = pm.getOnboardingConfig(); + console.log("Gnosis onboarding pre-set: maxOnboardingsPerAccount =", c.maxOnboardingsPerAccount); + + vm.startBroadcast(deployerKey); + // Re-set onboarding config, preserving all existing values and only adding the cap. + PoaManager(GNOSIS_POA_MANAGER) + .adminCall( + GNOSIS_PAYMASTER, + abi.encodeWithSignature( + "setOnboardingConfig(uint128,uint128,uint8,bool,address)", + c.maxGasPerCreation, + c.dailyCreationLimit, + MAX_ONBOARDINGS_PER_ACCOUNT, + c.enabled, + c.accountRegistry + ) + ); + vm.stopBroadcast(); + console.log("Gnosis onboarding cap set to", MAX_ONBOARDINGS_PER_ACCOUNT); + } +} + +/// @title Step4_SetCapArbitrum — activate the per-account cap on the Arbitrum paymaster (preserves other config). +/// Usage: FOUNDRY_PROFILE=production forge script .../:Step4_SetCapArbitrum --rpc-url arbitrum --broadcast --slow --optimizer-runs 200 +contract Step4_SetCapArbitrum is Script { + function run() public { + uint256 deployerKey = vm.envOr("PRIVATE_KEY", vm.envUint("DEPLOYER_PRIVATE_KEY")); + PaymasterHub pm = PaymasterHub(payable(ARB_PAYMASTER)); + PaymasterHub.OnboardingConfig memory c = pm.getOnboardingConfig(); + + vm.startBroadcast(deployerKey); + PoaManagerHub(payable(HUB)) + .adminCall( + ARB_PAYMASTER, + abi.encodeWithSignature( + "setOnboardingConfig(uint128,uint128,uint8,bool,address)", + c.maxGasPerCreation, + c.dailyCreationLimit, + MAX_ONBOARDINGS_PER_ACCOUNT, + c.enabled, + c.accountRegistry + ) + ); + vm.stopBroadcast(); + console.log("Arbitrum onboarding cap set to", MAX_ONBOARDINGS_PER_ACCOUNT); + } +} + +/** + * @title Sim_GnosisUpgrade + * @notice Fork-simulates the v18 upgrade + cap activation against LIVE Gnosis state and asserts: + * 1. The beacon upgrade preserves existing onboarding storage (maxGasPerCreation / dailyCreationLimit + * / accountRegistry unchanged), and the appended field reads 0 (= unlimited) so onboarding is NOT + * bricked by the upgrade alone. + * 2. After the admin sets the cap, getOnboardingConfig().maxOnboardingsPerAccount == 3. + * Validated PASS under FOUNDRY_PROFILE=production (broadcast-representative bytecode); also runs under + * the default profile. This is the real production-profile sim required before broadcast. + * + * Usage: forge script script/upgrades/UpgradePaymasterOnboardingCap.s.sol:Sim_GnosisUpgrade --fork-url gnosis -vvv + */ +contract Sim_GnosisUpgrade is Script { + function run() public { + PaymasterHub pm = PaymasterHub(payable(GNOSIS_PAYMASTER)); + PoaManager poa = PoaManager(GNOSIS_POA_MANAGER); + address owner = poa.owner(); + console.log("Gnosis PoaManager owner:", owner); + + // Capture live pre-upgrade onboarding config via low-level call decoding the OLD 6-field struct + // (the live impl predates the appended field, so the new ABI getter cannot decode its return). + (bool ok, bytes memory raw) = GNOSIS_PAYMASTER.staticcall(abi.encodeWithSignature("getOnboardingConfig()")); + require(ok, "Sim: pre-upgrade getOnboardingConfig() failed"); + ( + uint128 preMaxGas, + uint128 preDailyLimit,, // attemptsToday + , // currentDay + bool preEnabled, + address preRegistry + ) = abi.decode(raw, (uint128, uint128, uint128, uint32, bool, address)); + console.log("PRE maxGasPerCreation:", preMaxGas); + console.log("PRE dailyCreationLimit:", preDailyLimit); + console.log("PRE enabled:", preEnabled); + + // 1. Deploy the new impl and upgrade the Gnosis beacon (as the PoaManager owner would). + address newImpl = address(new PaymasterHub()); + vm.prank(owner); + poa.upgradeBeacon("PaymasterHub", newImpl, VERSION); + require(poa.getCurrentImplementationById(keccak256("PaymasterHub")) == newImpl, "Sim: beacon not upgraded"); + + // 2. Storage preserved + appended field reads 0 (= unlimited; onboarding not bricked by the upgrade). + PaymasterHub.OnboardingConfig memory mid = pm.getOnboardingConfig(); + require(mid.maxGasPerCreation == preMaxGas, "Sim: maxGasPerCreation drifted"); + require(mid.dailyCreationLimit == preDailyLimit, "Sim: dailyCreationLimit drifted"); + require(mid.accountRegistry == preRegistry, "Sim: accountRegistry drifted"); + require(mid.enabled == preEnabled, "Sim: enabled drifted"); + require(mid.maxOnboardingsPerAccount == 0, "Sim: appended field should read 0 (unlimited) pre-config"); + console.log("OK: upgrade preserved onboarding storage; cap defaults to 0 (unlimited)."); + + // 3. Activate the cap via the PoaManager admin path and assert it took effect. + vm.prank(owner); + poa.adminCall( + GNOSIS_PAYMASTER, + abi.encodeWithSignature( + "setOnboardingConfig(uint128,uint128,uint8,bool,address)", + mid.maxGasPerCreation, + mid.dailyCreationLimit, + MAX_ONBOARDINGS_PER_ACCOUNT, + mid.enabled, + mid.accountRegistry + ) + ); + PaymasterHub.OnboardingConfig memory post = pm.getOnboardingConfig(); + require(post.maxOnboardingsPerAccount == MAX_ONBOARDINGS_PER_ACCOUNT, "Sim: cap not applied"); + require(post.maxGasPerCreation == preMaxGas, "Sim: config clobbered while setting cap"); + require(post.dailyCreationLimit == preDailyLimit, "Sim: config clobbered while setting cap"); + + console.log("PASS: v18 upgrade + cap activation validated against live Gnosis state."); + console.log("POST maxOnboardingsPerAccount:", post.maxOnboardingsPerAccount); + } +} diff --git a/src/EligibilityModule.sol b/src/EligibilityModule.sol index e4bd649..3190407 100644 --- a/src/EligibilityModule.sol +++ b/src/EligibilityModule.sol @@ -1125,14 +1125,6 @@ contract EligibilityModule is Initializable, IHatsEligibility { } } - function _isEligible(uint8 flags) internal pure returns (bool) { - return (flags & ELIGIBLE_FLAG) != 0; - } - - function _hasGoodStanding(uint8 flags) internal pure returns (bool) { - return (flags & STANDING_FLAG) != 0; - } - function _isVouchingEnabled(uint8 flags) internal pure returns (bool) { return (flags & ENABLED_FLAG) != 0; } diff --git a/src/HybridVoting.sol b/src/HybridVoting.sol index 0170130..25fff23 100644 --- a/src/HybridVoting.sol +++ b/src/HybridVoting.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.30; /* OpenZeppelin v5.3 Upgradeables */ import "@openzeppelin-contracts-upgradeable/contracts/proxy/utils/Initializable.sol"; -import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import {IHats} from "lib/hats-protocol/src/Interfaces/IHats.sol"; import {IExecutor} from "./Executor.sol"; import {HatManager} from "./libs/HatManager.sol"; diff --git a/src/OrgDeployer.sol b/src/OrgDeployer.sol index 4bade7e..f02caec 100644 --- a/src/OrgDeployer.sol +++ b/src/OrgDeployer.sol @@ -7,7 +7,6 @@ import {IHats} from "@hats-protocol/src/Interfaces/IHats.sol"; import "./OrgRegistry.sol"; import {IHybridVotingInit} from "./libs/ModuleDeploymentLib.sol"; -import {RoleResolver} from "./libs/RoleResolver.sol"; import {GovernanceFactory, IHatsTreeSetup} from "./factories/GovernanceFactory.sol"; import {AccessFactory} from "./factories/AccessFactory.sol"; import {ModulesFactory} from "./factories/ModulesFactory.sol"; diff --git a/src/ParticipationToken.sol b/src/ParticipationToken.sol index 129c75a..56dc9c2 100644 --- a/src/ParticipationToken.sol +++ b/src/ParticipationToken.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.20; /*──────────────────── OpenZeppelin v5.3 Upgradeables ─────────────*/ import "@openzeppelin-contracts-upgradeable/contracts/token/ERC20/extensions/ERC20VotesUpgradeable.sol"; -import "@openzeppelin-contracts-upgradeable/contracts/utils/ContextUpgradeable.sol"; import "@openzeppelin-contracts-upgradeable/contracts/utils/ReentrancyGuardUpgradeable.sol"; /*────────────── External Hats interface ─────────────*/ diff --git a/src/PaymasterHub.sol b/src/PaymasterHub.sol index 9a50757..c0737b0 100644 --- a/src/PaymasterHub.sol +++ b/src/PaymasterHub.sol @@ -25,8 +25,6 @@ import {PaymasterCalldataLib} from "./libs/PaymasterCalldataLib.sol"; * @custom:security-contact security@poa.org */ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyGuardUpgradeable, IERC165 { - using UserOpLib for bytes32; - // ============ Constants ============ uint8 private constant PAYMASTER_DATA_VERSION = 1; uint8 private constant SUBJECT_TYPE_ACCOUNT = 0x00; @@ -111,6 +109,9 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG uint32 currentDay; // Day tracker (timestamp / 1 days) bool enabled; // Whether onboarding sponsorship is active address accountRegistry; // UniversalAccountRegistry — only allowed callData target during onboarding + // Appended (v-next): lifetime cap on sponsored onboardings per sender (0 == unlimited). + // Packs into the same slot as `accountRegistry` (160 + 8 bits) — no existing field moves. + uint8 maxOnboardingsPerAccount; } /** @@ -167,6 +168,8 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG keccak256(abi.encode(uint256(keccak256("poa.paymasterhub.orgdeploy")) - 1)); bytes32 private constant ORG_DEPLOY_COUNTS_STORAGE_LOCATION = keccak256(abi.encode(uint256(keccak256("poa.paymasterhub.orgdeploy.counts")) - 1)); + bytes32 private constant ONBOARDING_COUNTS_STORAGE_LOCATION = + keccak256(abi.encode(uint256(keccak256("poa.paymasterhub.onboarding.counts")) - 1)); // ============ Constructor ============ /// @custom:oz-upgrades-unsafe-allow constructor @@ -221,6 +224,7 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG OnboardingConfig storage onboarding = _getOnboardingStorage(); onboarding.maxGasPerCreation = 0.01 ether; // ~$30 worth of gas at typical L1 prices onboarding.dailyCreationLimit = 1000; // 1000 accounts per day + onboarding.maxOnboardingsPerAccount = 3; // lifetime sponsored onboardings per sender: register + profile + 1 retry (0 == unlimited) onboarding.enabled = true; // Initialize org deploy config (enabled, max cost per deploy, 100 deployments/day, 2 per account) @@ -1276,25 +1280,30 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG * @dev Only PoaManager can modify onboarding parameters * @param _maxGasPerCreation Maximum cost in wei allowed per account creation * @param _dailyCreationLimit Maximum accounts that can be created per day globally + * @param _maxOnboardingsPerAccount Lifetime cap on sponsored onboardings per sender (0 == unlimited) * @param _enabled Whether onboarding sponsorship is active * @param _accountRegistry UniversalAccountRegistry address (only allowed callData target) */ function setOnboardingConfig( uint128 _maxGasPerCreation, uint128 _dailyCreationLimit, + uint8 _maxOnboardingsPerAccount, bool _enabled, address _accountRegistry ) external { - if (msg.sender != _getMainStorage().poaManager) revert PaymasterHubErrors.NotPoaManager(); + if (msg.sender != _getMainStorage().poaManager) { + revert PaymasterHubErrors.NotPoaManager(); + } OnboardingConfig storage onboarding = _getOnboardingStorage(); onboarding.maxGasPerCreation = _maxGasPerCreation; onboarding.dailyCreationLimit = _dailyCreationLimit; + onboarding.maxOnboardingsPerAccount = _maxOnboardingsPerAccount; onboarding.enabled = _enabled; onboarding.accountRegistry = _accountRegistry; emit PaymasterHubErrors.OnboardingConfigUpdated( - _maxGasPerCreation, _dailyCreationLimit, _enabled, _accountRegistry + _maxGasPerCreation, _dailyCreationLimit, _maxOnboardingsPerAccount, _enabled, _accountRegistry ); } @@ -1488,6 +1497,13 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG } } + function _getOnboardingCountsStorage() private pure returns (mapping(address => uint8) storage $) { + bytes32 slot = ONBOARDING_COUNTS_STORAGE_LOCATION; + assembly { + $.slot := slot + } + } + function _getOrgDeployStorage() private pure returns (OrgDeployConfig storage $) { bytes32 slot = ORG_DEPLOY_STORAGE_LOCATION; assembly { @@ -1631,6 +1647,21 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG // Check gas cost limit if (maxCost > onboarding.maxGasPerCreation) revert PaymasterHubErrors.GasTooHigh(); + // Check per-account lifetime limit. Unlike the daily counter, this is NOT refunded in postOp on failure: + // a failed onboarding op still charges the solidarity fund (_updateOnboardingUsage deducts actualGasCost + // regardless of success), so every solidarity-charging attempt must consume the cap — otherwise repeated + // failing ops would drain the fund while keeping the count at zero. (Validation state only persists when the + // op is actually included on-chain, at which point postOp always runs and charges, so count and spend stay + // in lockstep.) maxOnboardingsPerAccount == 0 means unlimited; in that mode we skip counting entirely to avoid + // uint8 overflow on heavy senders and to keep already-deployed hubs (where the appended field reads 0) working. + if (onboarding.maxOnboardingsPerAccount != 0) { + mapping(address => uint8) storage counts = _getOnboardingCountsStorage(); + if (counts[account] >= onboarding.maxOnboardingsPerAccount) { + revert PaymasterHubErrors.OnboardingLimitExceeded(); + } + counts[account]++; + } + // Check daily rate limit uint32 today = uint32(block.timestamp / 1 days); if (today != onboarding.currentDay) { diff --git a/src/PaymasterHubLens.sol b/src/PaymasterHubLens.sol index dc04208..a184cbc 100644 --- a/src/PaymasterHubLens.sol +++ b/src/PaymasterHubLens.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.24; import {IPaymaster} from "./interfaces/IPaymaster.sol"; import {IEntryPoint} from "./interfaces/IEntryPoint.sol"; -import {PackedUserOperation, UserOpLib} from "./interfaces/PackedUserOperation.sol"; +import {PackedUserOperation} from "./interfaces/PackedUserOperation.sol"; import {IHats} from "lib/hats-protocol/src/Interfaces/IHats.sol"; import {IERC165} from "lib/openzeppelin-contracts/contracts/utils/introspection/IERC165.sol"; import {PaymasterHubErrors} from "./libs/PaymasterHubErrors.sol"; @@ -91,8 +91,6 @@ interface IPaymasterHubStorage { * All public functions take bytes32 orgId to match PaymasterHub's org-scoped storage. */ contract PaymasterHubLens { - using UserOpLib for bytes32; - // ============ Constants ============ uint8 private constant PAYMASTER_DATA_VERSION = 1; uint8 private constant SUBJECT_TYPE_ACCOUNT = 0x00; diff --git a/src/libs/BudgetLib.sol b/src/libs/BudgetLib.sol index 83e9919..1d3a3c5 100644 --- a/src/libs/BudgetLib.sol +++ b/src/libs/BudgetLib.sol @@ -10,7 +10,6 @@ library BudgetLib { /* ─────────── Errors ─────────── */ error BudgetExceeded(); error SpentUnderflow(); - error CapBelowCommitted(); /* ─────────── Constants ─────────── */ /// @notice Sentinel value meaning "unlimited budget" (no cap enforced). @@ -58,77 +57,4 @@ library BudgetLib { budget.spent -= uint128(delta); } } - - /** - * @notice Update budget spent amount (can increase or decrease) - * @param budget The budget struct to modify - * @param oldAmount Previous amount to rollback - * @param newAmount New amount to apply - * @param cap The budget cap (0 means unlimited) - */ - function updateSpent(Budget storage budget, uint256 oldAmount, uint256 newAmount, uint256 cap) internal { - // Rollback old amount - if (oldAmount > 0) { - subtractSpent(budget, oldAmount); - } - - // Apply new amount - if (newAmount > 0) { - addSpent(budget, newAmount, cap); - } - } - - /** - * @notice Update budget spent amount using budget's own cap - * @param budget The budget struct to modify - * @param oldAmount Previous amount to rollback - * @param newAmount New amount to apply - */ - function updateSpent(Budget storage budget, uint256 oldAmount, uint256 newAmount) internal { - updateSpent(budget, oldAmount, newAmount, budget.cap); - } - - /** - * @notice Check if an amount can be added without exceeding cap - * @param budget The budget struct to check - * @param delta Amount to potentially add - * @return bool True if the addition would not exceed cap - */ - function canAddSpent(Budget storage budget, uint256 delta) internal view returns (bool) { - if (budget.cap == UNLIMITED) return true; - return budget.spent + delta <= budget.cap; - } - - /** - * @notice Get remaining budget capacity - * @param budget The budget struct to check - * @return uint256 Remaining capacity (returns max uint256 if unlimited) - */ - function remainingCapacity(Budget storage budget) internal view returns (uint256) { - if (budget.cap == UNLIMITED) return type(uint256).max; - if (budget.spent >= budget.cap) return 0; - return budget.cap - budget.spent; - } - - /** - * @notice Check if a new cap is valid (not below current spent) - * @param budget The budget struct to check - * @param newCap The proposed new cap - * @return bool True if the new cap is valid - */ - function isValidCap(Budget storage budget, uint256 newCap) internal view returns (bool) { - // Disabled (0) and unlimited are always valid - if (newCap == 0 || newCap == UNLIMITED) return true; - return newCap >= budget.spent; - } - - /** - * @notice Set budget cap with validation - * @param budget The budget struct to modify - * @param newCap The new cap to set - */ - function setCap(Budget storage budget, uint256 newCap) internal { - if (!isValidCap(budget, newCap)) revert CapBelowCommitted(); - budget.cap = uint128(newCap); - } } diff --git a/src/libs/HatManager.sol b/src/libs/HatManager.sol index 2af656f..c888392 100644 --- a/src/libs/HatManager.sol +++ b/src/libs/HatManager.sol @@ -57,38 +57,6 @@ library HatManager { return _checkHatsBatch(hats, hatArray, user); } - /** - * @notice Check if a user wears any hat from a memory array - * @param hats The Hats Protocol contract - * @param hatArray Array of hat IDs to check - * @param user The user address to check - * @return bool True if user wears any hat from the array - */ - function hasAnyHatMemory(IHats hats, uint256[] memory hatArray, address user) internal view returns (bool) { - uint256 len = hatArray.length; - if (len == 0) return false; - if (len == 1) return hats.isWearerOfHat(user, hatArray[0]); - - // Build batch check arrays - address[] memory wearers = new address[](len); - for (uint256 i; i < len;) { - wearers[i] = user; - unchecked { - ++i; - } - } - - uint256[] memory balances = hats.balanceOfBatch(wearers, hatArray); - for (uint256 i; i < len;) { - if (balances[i] > 0) return true; - unchecked { - ++i; - } - } - - return false; - } - /** * @notice Check if a specific hat is in an array * @param hatArray Array of hat IDs to search @@ -147,61 +115,6 @@ library HatManager { return removedCount; } - /** - * @notice Efficiently check if user has specific hat without external calls - * @dev Use this when you already know the specific hat ID to check - * @param hats The Hats Protocol contract - * @param user The user address - * @param hatId The specific hat ID to check - * @return bool True if user wears the hat - */ - function hasSpecificHat(IHats hats, address user, uint256 hatId) internal view returns (bool) { - return hats.isWearerOfHat(user, hatId); - } - - /** - * @notice Batch add multiple hats to an array - * @param hatArray Array to add hats to - * @param hatIds Array of hat IDs to add - * @return addedCount Number of hats actually added (excluding duplicates) - */ - function addHatsBatch(uint256[] storage hatArray, uint256[] calldata hatIds) internal returns (uint256 addedCount) { - for (uint256 i; i < hatIds.length;) { - if (setHatInArray(hatArray, hatIds[i], true)) { - unchecked { - ++addedCount; - } - } - unchecked { - ++i; - } - } - return addedCount; - } - - /** - * @notice Batch remove multiple hats from an array - * @param hatArray Array to remove hats from - * @param hatIds Array of hat IDs to remove - * @return removedCount Number of hats actually removed - */ - function removeHatsBatch(uint256[] storage hatArray, uint256[] calldata hatIds) - internal - returns (uint256 removedCount) - { - for (uint256 i; i < hatIds.length;) { - if (setHatInArray(hatArray, hatIds[i], false)) { - unchecked { - ++removedCount; - } - } - unchecked { - ++i; - } - } - return removedCount; - } - /* ─────────── Internal Helpers ─────────── */ /** diff --git a/src/libs/HybridVotingCore.sol b/src/libs/HybridVotingCore.sol index 8a1121e..d56859f 100644 --- a/src/libs/HybridVotingCore.sol +++ b/src/libs/HybridVotingCore.sol @@ -5,7 +5,6 @@ import "../HybridVoting.sol"; import "./VotingErrors.sol"; import "./VotingMath.sol"; import {IExecutor} from "../Executor.sol"; -import {IHats} from "lib/hats-protocol/src/Interfaces/IHats.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; library HybridVotingCore { diff --git a/src/libs/HybridVotingProposals.sol b/src/libs/HybridVotingProposals.sol index 919655e..b4a3d01 100644 --- a/src/libs/HybridVotingProposals.sol +++ b/src/libs/HybridVotingProposals.sol @@ -4,10 +4,8 @@ pragma solidity ^0.8.30; import "../HybridVoting.sol"; import "./VotingErrors.sol"; import "./VotingMath.sol"; -import "./HatManager.sol"; import "./ValidationLib.sol"; import {IExecutor} from "../Executor.sol"; -import {IHats} from "lib/hats-protocol/src/Interfaces/IHats.sol"; library HybridVotingProposals { bytes32 private constant _STORAGE_SLOT = keccak256("poa.hybridvoting.v2.storage"); diff --git a/src/libs/PaymasterHubErrors.sol b/src/libs/PaymasterHubErrors.sol index f25569e..e4a3f17 100644 --- a/src/libs/PaymasterHubErrors.sol +++ b/src/libs/PaymasterHubErrors.sol @@ -42,9 +42,6 @@ library PaymasterHubErrors { /// @notice Rule ID is not a recognized rule type error InvalidRuleId(); - /// @notice ETH transfer to recipient failed - error PaymentFailed(); - /// @notice Subject type byte is not a recognized type (0x00, 0x01, 0x03, 0x04) error InvalidSubjectType(); @@ -111,6 +108,9 @@ library PaymasterHubErrors { /// @notice Onboarding sponsorship feature is disabled error OnboardingDisabled(); + /// @notice Account has reached its lifetime onboarding sponsorship limit + error OnboardingLimitExceeded(); + /// @notice Global daily onboarding creation limit has been reached error OnboardingDailyLimitExceeded(); @@ -166,17 +166,11 @@ library PaymasterHubErrors { /// @notice Emitted when ETH is deposited to the EntryPoint event DepositIncrease(uint256 amount, uint256 newDeposit); - /// @notice Emitted when ETH is withdrawn from the EntryPoint deposit - event DepositWithdraw(address indexed to, uint256 amount); - /// @notice Emitted when a subject's budget usage increases event UsageIncreased( bytes32 indexed orgId, bytes32 subjectKey, uint256 delta, uint128 usedInEpoch, uint32 epochStart ); - /// @notice Emitted on emergency withdrawal by PoaManager - event EmergencyWithdraw(address indexed to, uint256 amount); - /// @notice Emitted when ETH is deposited for a specific org event OrgDepositReceived(bytes32 indexed orgId, address indexed from, uint256 amount); @@ -199,7 +193,11 @@ library PaymasterHubErrors { /// @notice Emitted when onboarding configuration is updated event OnboardingConfigUpdated( - uint128 maxGasPerCreation, uint128 dailyCreationLimit, bool enabled, address accountRegistry + uint128 maxGasPerCreation, + uint128 dailyCreationLimit, + uint8 maxOnboardingsPerAccount, + bool enabled, + address accountRegistry ); /// @notice Emitted when a new account is created via onboarding sponsorship diff --git a/src/libs/RoleResolver.sol b/src/libs/RoleResolver.sol index 19eea0a..254ba80 100644 --- a/src/libs/RoleResolver.sol +++ b/src/libs/RoleResolver.sol @@ -29,31 +29,6 @@ library RoleResolver { } } - /** - * @notice Validates that all role indices are within bounds - * @param roleIndices Array of role indices to validate - * @param maxRoles Maximum number of roles in the organization - * @return valid True if all indices are valid - */ - function validateRoleIndices(uint256[] memory roleIndices, uint256 maxRoles) internal pure returns (bool valid) { - uint256 length = roleIndices.length; - for (uint256 i = 0; i < length; i++) { - if (roleIndices[i] >= maxRoles) { - return false; - } - } - return true; - } - - /** - * @notice Ensures array is not empty (for critical roles that must be assigned) - * @param roleIndices Array to check - * @return True if array has at least one element - */ - function requireNonEmpty(uint256[] memory roleIndices) internal pure returns (bool) { - return roleIndices.length > 0; - } - /** * @notice Resolves a bitmap of role indices to their corresponding Hat IDs * @dev Uses bitmap where bit N represents role index N (supports up to 256 roles) diff --git a/src/libs/VotingErrors.sol b/src/libs/VotingErrors.sol index e4f8d6f..99831ec 100644 --- a/src/libs/VotingErrors.sol +++ b/src/libs/VotingErrors.sol @@ -13,14 +13,12 @@ library VotingErrors { error TooManyOptions(); error TooManyCalls(); error ZeroAddress(); - error InvalidMetadata(); error RoleNotAllowed(); error WeightSumNot100(uint256 sum); error InvalidWeight(); error DuplicateIndex(); error TargetNotAllowed(); error TargetSelf(); - error InvalidTarget(); error EmptyBatch(); error InvalidThreshold(); error InvalidQuorum(); @@ -29,6 +27,5 @@ library VotingErrors { error InvalidClassCount(); error InvalidSliceSum(); error TooManyClasses(); - error InvalidStrategy(); error AlreadyExecuted(); } diff --git a/src/libs/VotingMath.sol b/src/libs/VotingMath.sol index c4dda8b..13bdbff 100644 --- a/src/libs/VotingMath.sol +++ b/src/libs/VotingMath.sol @@ -9,9 +9,6 @@ pragma solidity ^0.8.20; library VotingMath { /* ─────────── Errors ─────────── */ error InvalidThreshold(); - error InvalidSplit(); - error InvalidMinBalance(); - error MinBalanceNotMet(uint256 required); error RoleNotAllowed(); error DuplicateIndex(); error InvalidIndex(); @@ -50,31 +47,6 @@ library VotingMath { if (threshold == 0 || threshold > 100) revert InvalidThreshold(); } - /** - * @notice Validate split percentage - * @param split Split percentage (0-100) - */ - function validateSplit(uint8 split) internal pure { - if (split > 100) revert InvalidSplit(); - } - - /** - * @notice Validate minimum balance - * @param minBalance Minimum balance required - */ - function validateMinBalance(uint256 minBalance) internal pure { - if (minBalance == 0) revert InvalidMinBalance(); - } - - /** - * @notice Check if balance meets minimum requirement - * @param balance Current balance - * @param minBalance Minimum required balance - */ - function checkMinBalance(uint256 balance, uint256 minBalance) internal pure { - if (balance < minBalance) revert MinBalanceNotMet(minBalance); - } - /* ─────────── Weight Validation (New Struct Version) ─────────── */ /** @@ -139,17 +111,6 @@ library VotingMath { /* ─────────── Power Calculation Functions ─────────── */ - /** - * @notice Calculate voting power based on balance and quadratic setting (legacy) - * @param balance Token balance - * @param quadratic Whether to use quadratic voting - * @return power The calculated voting power - */ - function calculateVotingPower(uint256 balance, bool quadratic) internal pure returns (uint256 power) { - if (balance == 0) return 0; - return quadratic ? sqrt(balance) : balance; - } - /** * @notice Calculate voting power for participation token holders * @param bal Token balance @@ -183,32 +144,6 @@ library VotingMath { if (p > 0) ptRaw = p * 100; // match existing scaling } - /** - * @notice Calculate raw voting power for a voter (legacy) - * @param hasDemocracyHat Whether voter has democracy hat - * @param tokenBalance Token balance - * @param minBalance Minimum required balance - * @param quadratic Whether to use quadratic voting - * @return ddRaw Direct democracy raw power - * @return ptRaw Participation token raw power - */ - function calculateRawPowers(bool hasDemocracyHat, uint256 tokenBalance, uint256 minBalance, bool quadratic) - internal - pure - returns (uint256 ddRaw, uint256 ptRaw) - { - // Direct democracy power (only if has democracy hat) - ddRaw = hasDemocracyHat ? 100 : 0; - - // Participation token power - if (tokenBalance < minBalance) { - ptRaw = 0; - } else { - uint256 power = calculateVotingPower(tokenBalance, quadratic); - ptRaw = power * 100; // raw numerator - } - } - /* ─────────── Accumulation Helpers ─────────── */ /** @@ -436,20 +371,6 @@ library VotingMath { ok = thresholdMet && meetsMargin; } - /** - * @notice Validate class slices sum to 100 - * @param slices Array of slice percentages - */ - function validateClassSlices(uint8[] memory slices) internal pure { - if (slices.length == 0) revert InvalidSplit(); - uint256 sum; - for (uint256 i; i < slices.length; ++i) { - if (slices[i] == 0 || slices[i] > 100) revert InvalidSplit(); - sum += slices[i]; - } - if (sum != 100) revert InvalidSplit(); - } - /* ─────────── Math Utilities ─────────── */ /** @@ -473,14 +394,6 @@ library VotingMath { } } - /** - * @notice Check for overflow in uint128 - * @param value Value to check - */ - function checkOverflow(uint256 value) internal pure { - if (value > type(uint128).max) revert Overflow(); - } - /** * @notice Check if value fits in uint128 * @param value Value to check diff --git a/test/DeployerTest.t.sol b/test/DeployerTest.t.sol index 949ee27..896a4a1 100644 --- a/test/DeployerTest.t.sol +++ b/test/DeployerTest.t.sol @@ -1978,6 +1978,63 @@ contract DeployerTest is Test, IEligibilityModuleEvents { _assertWearingHat(candidate, setup.defaultRoleHat, true, "Candidate after claiming"); } + /// @notice H1 characterization test — locks the vouch-gating boundary and documents the exact misconfiguration + /// the frontend must reject (vouching.enabled && combineWithHierarchy && defaultEligible == true). + /// @dev This is a TEST-ONLY guard. There is intentionally NO on-chain contract change for H1: a properly + /// configured vouch-gated role (defaultEligible == false) is already un-self-claimable, and KUBI relies on + /// the combineWithHierarchy semantics, so we do not touch getWearerStatus/claimVouchedHat. The dangerous + /// combo is prevented in the UI (see the frontend role-config validation issue). If a future change ever + /// makes a default-not-eligible vouch-gated hat self-claimable, this test fails. + function testVouchGatingBoundaryAndComboMisconfig() public { + TestOrgSetup memory setup = _createTestOrg("Vouch Gating Boundary DAO"); + address stranger = address(0x5151); + address stranger2 = address(0x5252); + address voucher = voter1; + uint256 hat = setup.defaultRoleHat; + + _setupUserForVouching(setup.eligibilityModule, setup.exec, voucher); + _setupUserForVouching(setup.eligibilityModule, setup.exec, stranger); + _setupUserForVouching(setup.eligibilityModule, setup.exec, stranger2); + + // Delegate-shaped config: vouching on (quorum 1), combineWithHierarchy = TRUE, default NOT eligible. + // This mirrors DCP's live Delegate hat (vouching+combine, defaultEligible=false → gated). + _configureVouching(setup.eligibilityModule, setup.exec, hat, 1, setup.memberRoleHat, true, true); + _mintHat(setup.exec, setup.memberRoleHat, voucher); + + // (1) A stranger with zero vouches is NOT eligible and CANNOT self-claim — the gate holds. + _assertEligibilityStatus(setup.eligibilityModule, stranger, hat, false, false, "stranger before vouch"); + vm.prank(stranger); + vm.expectRevert(bytes("Not eligible to claim hat")); + EligibilityModule(setup.eligibilityModule).claimVouchedHat(hat); + + // (2) After meeting the vouch quorum, the same stranger becomes eligible and can claim. + _vouchFor(voucher, setup.eligibilityModule, stranger, hat); + _assertEligibilityStatus(setup.eligibilityModule, stranger, hat, true, true, "stranger after vouch"); + vm.prank(stranger); + EligibilityModule(setup.eligibilityModule).claimVouchedHat(hat); + _assertWearingHat(stranger, hat, true, "stranger after claim"); + + // (3) DOCUMENTED MISCONFIG the frontend must prevent: flipping defaultEligible to TRUE on this + // vouching+combine hat makes a brand-new stranger eligible with ZERO vouches — defeating the quorum. + _assertEligibilityStatus(setup.eligibilityModule, stranger2, hat, false, false, "stranger2 before flip"); + vm.prank(setup.exec); + EligibilityModule(setup.eligibilityModule).setDefaultEligibility(hat, true, true); // the dangerous flip + _assertEligibilityStatus( + setup.eligibilityModule, stranger2, hat, true, true, "stranger2 eligible with no vouches after flip" + ); + + // (4) Open-join path is unaffected: a hat with vouching DISABLED + defaultEligible=true is openly claimable + // (this is the intended Neighbor/QuickJoin behavior and must keep working). + uint256 openHat = setup.executiveRoleHat; + vm.prank(setup.exec); + EligibilityModule(setup.eligibilityModule).configureVouching(openHat, 0, 0, false); // quorum 0 => disabled + vm.prank(setup.exec); + EligibilityModule(setup.eligibilityModule).setDefaultEligibility(openHat, true, true); + _assertEligibilityStatus( + setup.eligibilityModule, address(0x9999), openHat, true, true, "open-join hat: anyone eligible" + ); + } + function testVouchingSystemHybridMode() public { TestOrgSetup memory setup = _createTestOrg("Hybrid Vouch Test DAO"); address candidate1 = address(0x201); @@ -6524,21 +6581,10 @@ contract DeployerTest is Test, IEligibilityModuleEvents { uint32[] memory epochLens = new uint32[](1); epochLens[0] = 366 days; // Above MAX_EPOCH_LENGTH (365 days) - PaymasterHub.DeployConfig memory config = PaymasterHub.DeployConfig({ - operatorHatId: 0, - maxFeePerGas: 0, - maxPriorityFeePerGas: 0, - maxCallGas: 0, - maxVerificationGas: 0, - maxPreVerificationGas: 0, - ruleTargets: new address[](0), - ruleSelectors: new bytes4[](0), - ruleAllowed: new bool[](0), - ruleMaxCallGasHints: new uint32[](0), - budgetSubjectKeys: keys, - budgetCapsPerEpoch: caps, - budgetEpochLens: epochLens - }); + PaymasterHub.DeployConfig memory config = _emptyDeployConfig(); + config.budgetSubjectKeys = keys; + config.budgetCapsPerEpoch = caps; + config.budgetEpochLens = epochLens; vm.prank(address(poaManager)); vm.expectRevert(abi.encodeWithSignature("InvalidEpochLength()")); @@ -6558,21 +6604,10 @@ contract DeployerTest is Test, IEligibilityModuleEvents { epochLens[0] = 1 days; epochLens[1] = 1 days; - PaymasterHub.DeployConfig memory config = PaymasterHub.DeployConfig({ - operatorHatId: 0, - maxFeePerGas: 0, - maxPriorityFeePerGas: 0, - maxCallGas: 0, - maxVerificationGas: 0, - maxPreVerificationGas: 0, - ruleTargets: new address[](0), - ruleSelectors: new bytes4[](0), - ruleAllowed: new bool[](0), - ruleMaxCallGasHints: new uint32[](0), - budgetSubjectKeys: keys, - budgetCapsPerEpoch: caps, - budgetEpochLens: epochLens - }); + PaymasterHub.DeployConfig memory config = _emptyDeployConfig(); + config.budgetSubjectKeys = keys; + config.budgetCapsPerEpoch = caps; + config.budgetEpochLens = epochLens; vm.prank(address(poaManager)); vm.expectRevert(abi.encodeWithSignature("ArrayLengthMismatch()")); @@ -6590,32 +6625,21 @@ contract DeployerTest is Test, IEligibilityModuleEvents { uint32[] memory epochLens = new uint32[](1); epochLens[0] = 30 minutes; // Below MIN_EPOCH_LENGTH (1 hour) - PaymasterHub.DeployConfig memory config = PaymasterHub.DeployConfig({ - operatorHatId: 0, - maxFeePerGas: 0, - maxPriorityFeePerGas: 0, - maxCallGas: 0, - maxVerificationGas: 0, - maxPreVerificationGas: 0, - ruleTargets: new address[](0), - ruleSelectors: new bytes4[](0), - ruleAllowed: new bool[](0), - ruleMaxCallGasHints: new uint32[](0), - budgetSubjectKeys: keys, - budgetCapsPerEpoch: caps, - budgetEpochLens: epochLens - }); + PaymasterHub.DeployConfig memory config = _emptyDeployConfig(); + config.budgetSubjectKeys = keys; + config.budgetCapsPerEpoch = caps; + config.budgetEpochLens = epochLens; vm.prank(address(poaManager)); vm.expectRevert(abi.encodeWithSignature("InvalidEpochLength()")); paymasterHub.registerAndConfigureOrg(orgId, 1, config); } - function testRegisterAndConfigureOrgUnauthorized() public { - // Non-registrar cannot call registerAndConfigureOrg directly - bytes32 orgId = keccak256("UNAUTH-ORG"); - - PaymasterHub.DeployConfig memory config = PaymasterHub.DeployConfig({ + /// @dev An empty PaymasterHub.DeployConfig. Extracted into its own frame so callers don't build the + /// 14-field struct (+ its abi-encoding) inline — that pushed `testRegisterAndConfigureOrgUnauthorized` + /// exactly 1 slot too deep under the production via-IR + optimizer profile. + function _emptyDeployConfig() internal pure returns (PaymasterHub.DeployConfig memory) { + return PaymasterHub.DeployConfig({ operatorHatId: 0, maxFeePerGas: 0, maxPriorityFeePerGas: 0, @@ -6630,6 +6654,13 @@ contract DeployerTest is Test, IEligibilityModuleEvents { budgetCapsPerEpoch: new uint128[](0), budgetEpochLens: new uint32[](0) }); + } + + function testRegisterAndConfigureOrgUnauthorized() public { + // Non-registrar cannot call registerAndConfigureOrg directly + bytes32 orgId = keccak256("UNAUTH-ORG"); + + PaymasterHub.DeployConfig memory config = _emptyDeployConfig(); // Random address should be rejected vm.prank(address(0xBEEF)); @@ -6647,21 +6678,11 @@ contract DeployerTest is Test, IEligibilityModuleEvents { bytes4[] memory sels = new bytes4[](1); // Length mismatch! sels[0] = bytes4(0x12345678); - PaymasterHub.DeployConfig memory config = PaymasterHub.DeployConfig({ - operatorHatId: 0, - maxFeePerGas: 0, - maxPriorityFeePerGas: 0, - maxCallGas: 0, - maxVerificationGas: 0, - maxPreVerificationGas: 0, - ruleTargets: targets, - ruleSelectors: sels, - ruleAllowed: new bool[](2), - ruleMaxCallGasHints: new uint32[](2), - budgetSubjectKeys: new bytes32[](0), - budgetCapsPerEpoch: new uint128[](0), - budgetEpochLens: new uint32[](0) - }); + PaymasterHub.DeployConfig memory config = _emptyDeployConfig(); + config.ruleTargets = targets; + config.ruleSelectors = sels; + config.ruleAllowed = new bool[](2); + config.ruleMaxCallGasHints = new uint32[](2); // Call as poaManager (authorized) vm.prank(address(poaManager)); diff --git a/test/HybridVotingSafeConfig.t.sol b/test/HybridVotingSafeConfig.t.sol new file mode 100644 index 0000000..5fd5cef --- /dev/null +++ b/test/HybridVotingSafeConfig.t.sol @@ -0,0 +1,314 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import "forge-std/Test.sol"; +import {StdInvariant} from "forge-std/StdInvariant.sol"; + +import {HybridVoting} from "../src/HybridVoting.sol"; +import {ParticipationToken} from "../src/ParticipationToken.sol"; +import {IExecutor} from "../src/Executor.sol"; +import {VotingErrors} from "../src/libs/VotingErrors.sol"; +import {MockHats} from "./mocks/MockHats.sol"; + +import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; + +/// @dev Minimal executor stub — proposals in these tests never need to execute a batch. +contract NoopExecutor is IExecutor { + function execute(uint256, Call[] calldata) external {} +} + +/** + * @title HybridVotingSafeConfig + * @notice H2: the audit flagged that HybridVoting's ERC20_BAL class reads a *live* `balanceOf` with no + * per-proposal snapshot (transfer-and-revote / mint-mid-proposal inflation). This is acceptable — + * and is left unchanged on-chain — *provided the org is configured safely*: the voting asset is the + * soulbound ParticipationToken and mint authority (TaskManager self-review / approvers) is not handed + * out broadly. These tests PROVE the safety boundary of a properly-configured org and document, with an + * executable example, the exact lever an unsafe config would expose. + */ +contract HybridVotingSafeConfigTest is Test { + address owner = vm.addr(1); + address alice = vm.addr(2); // member, 200 PT + address bob = vm.addr(3); // member, 100 PT + address sock = vm.addr(4); // member, 0 PT, NO mint authority (the would-be attacker) + address approver = vm.addr(5); // wears APPROVER hat + address taskManager = vm.addr(6); // the gated mint authority (stands in for TaskManager) + + uint256 constant MEMBER_HAT = 1; + uint256 constant APPROVER_HAT = 2; + uint256 constant CREATOR_HAT = 3; + + ParticipationToken public pt; + HybridVoting public hv; + MockHats hats; + NoopExecutor exec; + + function setUp() public { + hats = new MockHats(); + exec = new NoopExecutor(); + + hats.mintHat(MEMBER_HAT, alice); + hats.mintHat(MEMBER_HAT, bob); + hats.mintHat(MEMBER_HAT, sock); + hats.mintHat(MEMBER_HAT, approver); + hats.mintHat(APPROVER_HAT, approver); + hats.mintHat(CREATOR_HAT, alice); + + // Deploy the real (soulbound) ParticipationToken; narrow mint authority to `taskManager`. + pt = _deployPT(); + pt.setTaskManager(taskManager); + vm.startPrank(taskManager); + pt.mint(alice, 200 ether); + pt.mint(bob, 100 ether); + vm.stopPrank(); + + // Deploy HybridVoting with a single ERC20_BAL class (100%) over the soulbound PT. + hv = _deployHV(address(pt)); + } + + /* ─────────────────────────── helpers ─────────────────────────── */ + + /// @dev Split out of setUp() to keep each function's stack frame small (production via-IR profile). + function _deployPT() internal returns (ParticipationToken) { + uint256[] memory memberHats = new uint256[](1); + memberHats[0] = MEMBER_HAT; + uint256[] memory approverHats = new uint256[](1); + approverHats[0] = APPROVER_HAT; + UpgradeableBeacon ptBeacon = new UpgradeableBeacon(address(new ParticipationToken()), owner); + bytes memory ptInit = abi.encodeCall( + ParticipationToken.initialize, + (address(exec), "Participation", "PT", address(hats), memberHats, approverHats) + ); + return ParticipationToken(address(new BeaconProxy(address(ptBeacon), ptInit))); + } + + /// @dev Split out of setUp() to keep each function's stack frame small (production via-IR profile). + function _deployHV(address asset) internal returns (HybridVoting) { + uint256[] memory votingHats = new uint256[](1); + votingHats[0] = MEMBER_HAT; + uint256[] memory creatorHats = new uint256[](1); + creatorHats[0] = CREATOR_HAT; + address[] memory targets = new address[](1); + targets[0] = address(0xCA11); + HybridVoting.ClassConfig[] memory classes = new HybridVoting.ClassConfig[](1); + classes[0] = HybridVoting.ClassConfig({ + strategy: HybridVoting.ClassStrategy.ERC20_BAL, + slicePct: 100, + quadratic: false, + minBalance: 1 ether, + asset: asset, + hatIds: votingHats + }); + bytes memory initData = abi.encodeCall( + HybridVoting.initialize, (address(hats), address(exec), creatorHats, targets, uint8(50), classes) + ); + UpgradeableBeacon hvBeacon = new UpgradeableBeacon(address(new HybridVoting()), owner); + return HybridVoting(payable(address(new BeaconProxy(address(hvBeacon), initData)))); + } + + function _createPoll() internal returns (uint256 id) { + IExecutor.Call[][] memory batches = new IExecutor.Call[][](2); + batches[0] = new IExecutor.Call[](1); + batches[1] = new IExecutor.Call[](1); + batches[0][0] = IExecutor.Call({target: address(0xCA11), value: 0, data: ""}); + batches[1][0] = IExecutor.Call({target: address(0xCA11), value: 0, data: ""}); + uint256[] memory pollHats = new uint256[](0); // open to all MEMBER_HAT wearers + vm.prank(alice); + hv.createProposal(bytes("Treasury Spend"), bytes32(0), 30, 2, batches, pollHats); + id = hv.proposalsCount() - 1; + } + + function _vote(address who, uint256 id, uint8 option) internal { + uint8[] memory idx = new uint8[](1); + idx[0] = option; // 0 = YES, 1 = NO + uint8[] memory w = new uint8[](1); + w[0] = 100; + vm.prank(who); + hv.vote(id, idx, w); + } + + /* ───────────────────── Property 1: soulbound token blocks transfer-revote ───────────────────── */ + + /// @notice The transferable-asset inflation vector (transfer tokens between addresses to double-count) + /// is structurally impossible for the default config because PT is soulbound. + function test_PT_IsSoulbound_TransferRevoteImpossible() public { + vm.prank(alice); + vm.expectRevert(ParticipationToken.TransfersDisabled.selector); + pt.transfer(sock, 50 ether); + + vm.prank(alice); + vm.expectRevert(ParticipationToken.TransfersDisabled.selector); + pt.transferFrom(alice, sock, 50 ether); + + // delegation is also locked, so weight cannot be re-pointed to another voter + vm.prank(alice); + vm.expectRevert(ParticipationToken.TransfersDisabled.selector); + pt.delegate(sock); + } + + /* ───────────────────── Property 2: unprivileged actor cannot mint to inflate ───────────────────── */ + + /// @notice An ordinary member (no TaskManager / approver authority) cannot mint PT, and an approver cannot + /// self-approve their own request — so a voter's balance (hence ERC20_BAL weight) is fixed by what + /// they legitimately earned. + function test_UnprivilegedActor_CannotSelfMint() public { + // direct mint is gated to TaskManager / EducationHub / executor + vm.prank(sock); + vm.expectRevert(ParticipationToken.NotTaskOrEdu.selector); + pt.mint(sock, 1_000_000 ether); + + // the request flow does not let an approver mint to themselves + vm.prank(approver); + pt.requestTokens(uint96(1_000_000 ether), "ipfs://self"); + uint256 selfReqId = 1; + vm.prank(approver); + vm.expectRevert(ParticipationToken.NotRequester.selector); + pt.approveRequest(selfReqId); + + // a non-member cannot even open a request + vm.prank(address(0xDEAD)); + vm.expectRevert(ParticipationToken.NotMember.selector); + pt.requestTokens(uint96(1 ether), "ipfs://x"); + } + + /* ───────────────────── Scenario: unprivileged actor cannot flip a decided outcome ───────────────────── */ + + /// @notice With a safe config, a decided proposal cannot be flipped by an actor who lacks legitimate weight. + function test_SafeConfig_DecidedOutcomeCannotBeFlipped() public { + uint256 id = _createPoll(); + + _vote(alice, id, 0); // YES, 200 PT + _vote(bob, id, 1); // NO, 100 PT + + // `sock` is a member but holds 0 PT and has no way to acquire any (mint gated, transfers blocked). + // The contract's own Sybil guard rejects a zero-power voter outright — sock cannot even cast a ballot, + // so it can neither shift weight nor inflate the voter/quorum count. + uint8[] memory idx = new uint8[](1); + idx[0] = 1; + uint8[] memory w = new uint8[](1); + w[0] = 100; + vm.prank(sock); + vm.expectRevert(VotingErrors.Unauthorized.selector); + hv.vote(id, idx, w); + + vm.warp(block.timestamp + 31 minutes); + (uint256 winner, bool valid) = hv.announceWinner(id); + assertTrue(valid, "proposal should be valid"); + assertEq(winner, 0, "YES (200 PT) must win over NO (100 PT) despite sock's weightless vote"); + } + + /* ───────────────────── Boundary: the lever an UNSAFE config would expose ───────────────────── */ + + /// @notice Documents (executably) WHY the configuration matters: because weight is read live, an actor who + /// *does* hold mint authority can raise their own balance mid-proposal and thereby their vote weight. + /// A properly-configured org keeps this authority narrow (no broad self-review); this test shows the + /// lever exists so the safe-config requirement is concrete rather than assumed. + function test_Boundary_MintAuthorityIsTheLever() public { + uint256 id = _createPoll(); + + // Suppose `sock` were (mis)granted mint authority — stand in for a broadly-granted TaskManager self-review. + // We simulate by minting to sock through the authority, then sock votes with the inflated balance. + uint256 sockBefore = pt.balanceOf(sock); + vm.prank(taskManager); + pt.mint(sock, 1000 ether); // the inflation a safe config must prevent by restricting this authority + assertGt(pt.balanceOf(sock), sockBefore, "mint authority can raise balance mid-proposal (live weight)"); + + _vote(alice, id, 0); // YES, 200 PT + _vote(sock, id, 1); // NO, 1000 PT (only possible because of the unsafe mint authority) + + vm.warp(block.timestamp + 31 minutes); + (uint256 winner,) = hv.announceWinner(id); + assertEq(winner, 1, "with mint authority, NO flips the outcome - exactly what safe config must prevent"); + } +} + +/* ════════════════════════════════ Invariant: no unprivileged inflation ════════════════════════════════ */ + +/// @dev Handler exercising ONLY unprivileged actions (voting, and reverting transfer/mint attempts). +contract UnprivilegedActorHandler is Test { + ParticipationToken public pt; + HybridVoting public hv; + uint256 public proposalId; + address[] public actors; + + constructor(ParticipationToken _pt, HybridVoting _hv, uint256 _proposalId, address[] memory _actors) { + pt = _pt; + hv = _hv; + proposalId = _proposalId; + actors = _actors; + } + + function vote(uint256 actorSeed, uint8 option) external { + address a = actors[actorSeed % actors.length]; + uint8[] memory idx = new uint8[](1); + idx[0] = option % 2; + uint8[] memory w = new uint8[](1); + w[0] = 100; + vm.prank(a); + try hv.vote(proposalId, idx, w) {} catch {} + } + + function attemptTransfer(uint256 actorSeed, uint256 amount) external { + address a = actors[actorSeed % actors.length]; + address to = actors[(actorSeed + 1) % actors.length]; + vm.prank(a); + try pt.transfer(to, amount) {} catch {} + } + + function attemptSelfMint(uint256 actorSeed, uint256 amount) external { + address a = actors[actorSeed % actors.length]; + vm.prank(a); + try pt.mint(a, amount) {} catch {} + } +} + +contract HybridVotingSafeConfigInvariant is StdInvariant, Test { + HybridVotingSafeConfigTest internal env; + ParticipationToken internal pt; + HybridVoting internal hv; + UnprivilegedActorHandler internal handler; + uint256 internal initialSupply; + + function setUp() public { + // Reuse the safe-config deployment from the scenario test. + env = new HybridVotingSafeConfigTest(); + env.setUp(); + pt = env.pt(); + hv = env.hv(); + initialSupply = pt.totalSupply(); + + // Open a proposal as the creator (alice = vm.addr(2)) so the handler can fuzz votes against it. + address creator = vm.addr(2); + IExecutor.Call[][] memory batches = new IExecutor.Call[][](2); + batches[0] = new IExecutor.Call[](1); + batches[1] = new IExecutor.Call[](1); + batches[0][0] = IExecutor.Call({target: address(0xCA11), value: 0, data: ""}); + batches[1][0] = IExecutor.Call({target: address(0xCA11), value: 0, data: ""}); + uint256[] memory pollHats = new uint256[](0); + vm.prank(creator); + hv.createProposal(bytes("Invariant Poll"), bytes32(0), 60, 2, batches, pollHats); + uint256 pid = hv.proposalsCount() - 1; + + address[] memory actors = new address[](3); + actors[0] = vm.addr(2); // alice + actors[1] = vm.addr(3); // bob + actors[2] = vm.addr(4); // sock + handler = new UnprivilegedActorHandler(pt, hv, pid, actors); + targetContract(address(handler)); + } + + /// @notice No sequence of unprivileged voting / transfer / mint attempts can inflate the PT supply. + /// Because PT is soulbound and mint is gated, the legitimate weight distribution is immutable, + /// so the live-`balanceOf` tally cannot be gamed under a safe config. + function invariant_supplyConstantUnderUnprivilegedActivity() public view { + assertEq(pt.totalSupply(), initialSupply, "unprivileged activity must never change PT supply"); + } + + /// @notice Individual voter balances are likewise immutable under unprivileged activity. + function invariant_voterBalancesImmutable() public view { + assertEq(pt.balanceOf(vm.addr(2)), 200 ether, "alice balance must be fixed"); + assertEq(pt.balanceOf(vm.addr(3)), 100 ether, "bob balance must be fixed"); + assertEq(pt.balanceOf(vm.addr(4)), 0, "sock must never acquire weight"); + } +} diff --git a/test/PaymasterHubSolidarity.t.sol b/test/PaymasterHubSolidarity.t.sol index bf5e1af..c12513e 100644 --- a/test/PaymasterHubSolidarity.t.sol +++ b/test/PaymasterHubSolidarity.t.sol @@ -1361,7 +1361,7 @@ contract PaymasterHubSolidarityTest is Test { function testOnboardingRejectsNonZeroOrgId() public { hub.donateToSolidarity{value: 1 ether}(); vm.prank(poaManager); - hub.setOnboardingConfig(uint128(MAX_COST), 10, true, address(0)); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 0, true, address(0)); bytes memory pmData = _buildPaymasterData(ORG_ALPHA, SUBJECT_TYPE_POA_ONBOARDING, bytes32(0), RULE_ID_GENERIC); PackedUserOperation memory userOp = _buildUserOp(address(0xdead), "", pmData); userOp.initCode = hex"01"; @@ -1380,7 +1380,7 @@ contract PaymasterHubSolidarityTest is Test { vm.prank(poaManager); hub.unpauseSolidarityDistribution(); vm.prank(poaManager); - hub.setOnboardingConfig(uint128(MAX_COST), 10, true, address(0)); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 0, true, address(0)); address deployed = address(new DummySender()); bytes memory pmData = _buildPaymasterData(bytes32(0), SUBJECT_TYPE_POA_ONBOARDING, bytes32(0), RULE_ID_GENERIC); PackedUserOperation memory userOp = _buildUserOp(deployed, "", pmData); @@ -1396,7 +1396,7 @@ contract PaymasterHubSolidarityTest is Test { vm.prank(poaManager); hub.unpauseSolidarityDistribution(); vm.prank(poaManager); - hub.setOnboardingConfig(uint128(MAX_COST), 10, true, address(0)); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 0, true, address(0)); address newAccount = address(0xbeef); bytes memory pmData = _buildPaymasterData(bytes32(0), SUBJECT_TYPE_POA_ONBOARDING, bytes32(0), RULE_ID_GENERIC); PackedUserOperation memory userOp = _buildUserOp(newAccount, "", pmData); @@ -1421,7 +1421,7 @@ contract PaymasterHubSolidarityTest is Test { vm.prank(poaManager); hub.unpauseSolidarityDistribution(); vm.prank(poaManager); - hub.setOnboardingConfig(uint128(MAX_COST), 1, true, address(0)); + hub.setOnboardingConfig(uint128(MAX_COST), 1, 0, true, address(0)); address account1 = address(0xaa01); bytes memory pmData1 = _buildPaymasterData(bytes32(0), SUBJECT_TYPE_POA_ONBOARDING, bytes32(0), RULE_ID_GENERIC); PackedUserOperation memory userOp1 = _buildUserOp(account1, "", pmData1); @@ -1451,6 +1451,113 @@ contract PaymasterHubSolidarityTest is Test { hub.validatePaymasterUserOp(userOp3, keccak256("hash3"), MAX_COST); } + /*═══════════════════════ H4: per-account onboarding cap ═══════════════════════*/ + + /// @dev Build a minimal onboarding userOp for `account` (empty callData + initCode satisfies the + /// registerAccount path; registry is address(0) so no callData parsing is required). + function _onboardingOp(address account) internal view returns (PackedUserOperation memory userOp) { + bytes memory pmData = _buildPaymasterData(bytes32(0), SUBJECT_TYPE_POA_ONBOARDING, bytes32(0), RULE_ID_GENERIC); + userOp = _buildUserOp(account, "", pmData); + userOp.initCode = hex"01"; + } + + /// @notice A single sender cannot exceed its lifetime onboarding cap. + function testOnboardingPerAccountCapEnforced() public { + hub.donateToSolidarity{value: 1 ether}(); + vm.prank(poaManager); + hub.unpauseSolidarityDistribution(); + vm.prank(poaManager); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 1, true, address(0)); // cap = 1 per account + + address acct = address(0xbeef); + PackedUserOperation memory userOp = _onboardingOp(acct); + + // first onboarding for this sender succeeds + vm.prank(address(entryPoint)); + hub.validatePaymasterUserOp(userOp, keccak256("h1"), MAX_COST); + + // second from the SAME sender is rejected by the per-account cap (daily limit is 10, so it's not that) + vm.prank(address(entryPoint)); + vm.expectRevert(PaymasterHubErrors.OnboardingLimitExceeded.selector); + hub.validatePaymasterUserOp(userOp, keccak256("h2"), MAX_COST); + } + + /// @notice maxOnboardingsPerAccount == 0 disables the per-account cap (unlimited) — preserves legacy behavior. + function testOnboardingCapZeroMeansUnlimited() public { + hub.donateToSolidarity{value: 1 ether}(); + vm.prank(poaManager); + hub.unpauseSolidarityDistribution(); + vm.prank(poaManager); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 0, true, address(0)); // cap = 0 (unlimited) + + address acct = address(0xbeef); + PackedUserOperation memory userOp = _onboardingOp(acct); + + // repeated onboardings from the same sender all succeed (bounded only by the global daily limit) + for (uint256 i = 0; i < 4; i++) { + vm.prank(address(entryPoint)); + hub.validatePaymasterUserOp(userOp, keccak256(abi.encode("h", i)), MAX_COST); + } + } + + /// @notice The cap is tracked per sender — exhausting account A does not affect account B. + function testOnboardingCapIsolatedPerAccount() public { + hub.donateToSolidarity{value: 1 ether}(); + vm.prank(poaManager); + hub.unpauseSolidarityDistribution(); + vm.prank(poaManager); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 1, true, address(0)); // cap = 1 + + PackedUserOperation memory opA = _onboardingOp(address(0xA11CE)); + PackedUserOperation memory opB = _onboardingOp(address(0xB0B)); + + // A uses its single slot then is blocked + vm.prank(address(entryPoint)); + hub.validatePaymasterUserOp(opA, keccak256("a1"), MAX_COST); + vm.prank(address(entryPoint)); + vm.expectRevert(PaymasterHubErrors.OnboardingLimitExceeded.selector); + hub.validatePaymasterUserOp(opA, keccak256("a2"), MAX_COST); + + // B is unaffected and can still onboard + vm.prank(address(entryPoint)); + hub.validatePaymasterUserOp(opB, keccak256("b1"), MAX_COST); + } + + /// @notice Security property: a FAILED (reverted) onboarding op still consumes the per-account cap. + /// Unlike the daily throttle (which is refunded in postOp), the lifetime cap must NOT be refunded, + /// because a reverted op still charges the solidarity fund — refunding would let an attacker drain + /// the fund via repeated failing ops while keeping their count at zero. + function testOnboardingFailedOpStillConsumesPerAccountCap() public { + hub.donateToSolidarity{value: 1 ether}(); + vm.prank(poaManager); + hub.unpauseSolidarityDistribution(); + vm.prank(poaManager); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 1, true, address(0)); // cap = 1 + + address acct = address(0xbeef); + PackedUserOperation memory userOp = _onboardingOp(acct); + + // validate (count -> 1) then simulate a reverted execution via postOp + vm.prank(address(entryPoint)); + (bytes memory ctx,) = hub.validatePaymasterUserOp(userOp, keccak256("h1"), MAX_COST); + vm.prank(address(entryPoint)); + hub.postOp(IPaymaster.PostOpMode.opReverted, ctx, 50_000, 1); + + // the same sender is STILL blocked — the failed op consumed the cap (no refund) + vm.prank(address(entryPoint)); + vm.expectRevert(PaymasterHubErrors.OnboardingLimitExceeded.selector); + hub.validatePaymasterUserOp(userOp, keccak256("h2"), MAX_COST); + } + + /// @notice setOnboardingConfig persists the per-account cap and it is reflected by the getter. + function testSetOnboardingConfigSetsPerAccountCap() public { + vm.prank(poaManager); + hub.setOnboardingConfig(uint128(MAX_COST), 5, 7, true, address(0xABCD)); + assertEq(hub.getOnboardingConfig().maxOnboardingsPerAccount, 7); + assertEq(hub.getOnboardingConfig().dailyCreationLimit, 5); + assertTrue(hub.getOnboardingConfig().enabled); + } + /// @notice Onboarding accepts valid registerAccount callData function testOnboardingAcceptsRegisterAccountCallData() public { address registry = address(0xCC); @@ -1458,7 +1565,7 @@ contract PaymasterHubSolidarityTest is Test { vm.prank(poaManager); hub.unpauseSolidarityDistribution(); vm.prank(poaManager); - hub.setOnboardingConfig(uint128(MAX_COST), 10, true, registry); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 0, true, registry); address deployed = address(new DummySender()); bytes memory pmData = _buildPaymasterData(bytes32(0), SUBJECT_TYPE_POA_ONBOARDING, bytes32(0), RULE_ID_GENERIC); // Build execute(registryAddress, 0, registerAccount("alice")) @@ -1478,7 +1585,7 @@ contract PaymasterHubSolidarityTest is Test { vm.prank(poaManager); hub.unpauseSolidarityDistribution(); vm.prank(poaManager); - hub.setOnboardingConfig(uint128(MAX_COST), 10, true, registry); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 0, true, registry); address deployed = address(new DummySender()); bytes memory pmData = _buildPaymasterData(bytes32(0), SUBJECT_TYPE_POA_ONBOARDING, bytes32(0), RULE_ID_GENERIC); // Build execute(wrongTarget, 0, registerAccount("alice")) @@ -1498,7 +1605,7 @@ contract PaymasterHubSolidarityTest is Test { vm.prank(poaManager); hub.unpauseSolidarityDistribution(); vm.prank(poaManager); - hub.setOnboardingConfig(uint128(MAX_COST), 10, true, registry); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 0, true, registry); address deployed = address(new DummySender()); bytes memory pmData = _buildPaymasterData(bytes32(0), SUBJECT_TYPE_POA_ONBOARDING, bytes32(0), RULE_ID_GENERIC); // Build execute(registry, 0, someOtherFunction("data")) @@ -1518,7 +1625,7 @@ contract PaymasterHubSolidarityTest is Test { vm.prank(poaManager); hub.unpauseSolidarityDistribution(); vm.prank(poaManager); - hub.setOnboardingConfig(uint128(MAX_COST), 10, true, registry); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 0, true, registry); address deployed = address(new DummySender()); bytes memory pmData = _buildPaymasterData(bytes32(0), SUBJECT_TYPE_POA_ONBOARDING, bytes32(0), RULE_ID_GENERIC); // Build arbitrary callData (not execute()) @@ -1952,7 +2059,7 @@ contract PaymasterHubSolidarityTest is Test { vm.prank(poaManager); hub.unpauseSolidarityDistribution(); vm.prank(poaManager); - hub.setOnboardingConfig(uint128(MAX_COST), 10, true, address(0)); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 0, true, address(0)); address newAccount = address(0xbeef); bytes memory pmData = _buildPaymasterData(bytes32(0), SUBJECT_TYPE_POA_ONBOARDING, bytes32(0), RULE_ID_GENERIC); @@ -2016,7 +2123,7 @@ contract PaymasterHubSolidarityTest is Test { vm.prank(poaManager); hub.unpauseSolidarityDistribution(); vm.prank(poaManager); - hub.setOnboardingConfig(uint128(MAX_COST), 10, true, address(0)); + hub.setOnboardingConfig(uint128(MAX_COST), 10, 0, true, address(0)); address newAccount = address(0xbeef); bytes memory pmData = _buildPaymasterData(bytes32(0), SUBJECT_TYPE_POA_ONBOARDING, bytes32(0), RULE_ID_GENERIC);