diff --git a/src/interfaces/IB20.sol b/src/interfaces/IB20.sol index 6901ecca..9de40a05 100644 --- a/src/interfaces/IB20.sol +++ b/src/interfaces/IB20.sol @@ -469,15 +469,18 @@ interface IB20 { /// @notice Allowance granted by `owner` to `spender`. function allowance(address owner, address spender) external view returns (uint256); - /// @notice Transfers `amount` from `msg.sender` to `to`. Reverts with: - /// - `ContractPaused(TRANSFER)` if `TRANSFER` is paused. - /// - `PolicyForbids(TRANSFER_SENDER_POLICY, policyId)` if `msg.sender` - /// is not authorized under the active `TRANSFER_SENDER_POLICY` policy. - /// - `PolicyForbids(TRANSFER_RECEIVER_POLICY, policyId)` if `to` is not - /// authorized under the active `TRANSFER_RECEIVER_POLICY` policy. - /// - `InsufficientBalance(msg.sender, balance, amount)` if the - /// caller does not have enough balance. - /// - `InvalidReceiver(to)` if `to == address(0)`. + /// @notice Transfers `amount` from `msg.sender` to `to`. Preconditions + /// evaluate in the order listed; when multiple are violated, + /// the earliest-listed revert is the one that surfaces: + /// 1. `ContractPaused(TRANSFER)` if `TRANSFER` is paused. + /// 2. `InvalidReceiver(to)` if `to == address(0)`. + /// 3. `InvalidSender(msg.sender)` if `msg.sender == address(0)`. + /// 4. `PolicyForbids(TRANSFER_SENDER_POLICY, policyId)` if `msg.sender` + /// is not authorized under the active `TRANSFER_SENDER_POLICY`. + /// 5. `PolicyForbids(TRANSFER_RECEIVER_POLICY, policyId)` if `to` + /// is not authorized under the active `TRANSFER_RECEIVER_POLICY`. + /// 6. `InsufficientBalance(msg.sender, balance, amount)` if the + /// caller does not have enough balance. /// @dev Does NOT consult the `TRANSFER_EXECUTOR_POLICY` policy: on direct /// `transfer` the executor IS the sender, and the sender /// check already covers that address. When the token is @@ -486,13 +489,23 @@ interface IB20 { function transfer(address to, uint256 amount) external returns (bool); /// @notice Transfers `amount` from `from` to `to` using `msg.sender`'s - /// allowance. Reverts as `transfer` does, plus: - /// - `InsufficientAllowance(msg.sender, allowance, amount)` - /// if the caller does not have enough allowance from `from`. - /// - `InvalidSender(from)` if `from == address(0)`. - /// - `PolicyForbids(TRANSFER_EXECUTOR_POLICY, policyId)` if - /// `msg.sender != from` and `msg.sender` is not authorized - /// under the active `TRANSFER_EXECUTOR_POLICY` policy. + /// allowance. Preconditions evaluate in the order listed; + /// when multiple are violated, the earliest-listed revert is + /// the one that surfaces: + /// 1. `ContractPaused(TRANSFER)` if `TRANSFER` is paused. + /// 2. `InvalidReceiver(to)` if `to == address(0)`. + /// 3. `InvalidSender(from)` if `from == address(0)`. + /// 4. `InsufficientAllowance(msg.sender, allowance, amount)` + /// if the caller does not have enough allowance from `from`. + /// 5. `PolicyForbids(TRANSFER_EXECUTOR_POLICY, policyId)` if + /// `msg.sender != from` and `msg.sender` is not authorized + /// under the active `TRANSFER_EXECUTOR_POLICY`. + /// 6. `PolicyForbids(TRANSFER_SENDER_POLICY, policyId)` if `from` + /// is not authorized under the active `TRANSFER_SENDER_POLICY`. + /// 7. `PolicyForbids(TRANSFER_RECEIVER_POLICY, policyId)` if `to` + /// is not authorized under the active `TRANSFER_RECEIVER_POLICY`. + /// 8. `InsufficientBalance(from, balance, amount)` if `from` + /// does not have enough balance. /// @dev The sender-side check is performed against `from` (the /// party whose balance moves), the receiver check against /// `to`, and the executor check against `msg.sender` only @@ -502,11 +515,12 @@ interface IB20 { /// @notice Sets `spender`'s allowance to `amount`. NOT gated by any /// policy or by pause; only the act of MOVING balance is gated. - /// @dev Reverts with `InvalidApprover(msg.sender)` if the - /// caller is `address(0)` (theoretically unreachable for - /// normal callers but enforced for parity with OZ ERC20), - /// and `InvalidSpender(spender)` if - /// `spender == address(0)`. + /// @dev Reverts in the following canonical order (first violated + /// check wins): + /// 1. `InvalidApprover(msg.sender)` if the caller is + /// `address(0)` (theoretically unreachable for normal + /// callers but enforced for parity with OZ ERC20). + /// 2. `InvalidSpender(spender)` if `spender == address(0)`. function approve(address spender, uint256 amount) external returns (bool); /*////////////////////////////////////////////////////////////// @@ -558,12 +572,17 @@ interface IB20 { MINT / BURN //////////////////////////////////////////////////////////////*/ - /// @notice Mints `amount` to `to`. Requires `MINT_ROLE`. Subject to: - /// 1. `totalSupply + amount <= supplyCap` (else - /// `SupplyCapExceeded`). - /// 2. `MINT` is not paused (else `ContractPaused(MINT)`). - /// 3. `to` is authorized under the active `MINT_RECEIVER_POLICY` - /// policy (else `PolicyForbids(MINT_RECEIVER_POLICY, policyId)`). + /// @notice Mints `amount` to `to`. Preconditions evaluate in the + /// order listed; when multiple are violated, the + /// earliest-listed revert is the one that surfaces: + /// 1. `ContractPaused(MINT)` if `MINT` is paused. + /// 2. `AccessControlUnauthorizedAccount(msg.sender, MINT_ROLE)` + /// if the caller does not hold `MINT_ROLE`. + /// 3. `InvalidReceiver(to)` if `to == address(0)`. + /// 4. `PolicyForbids(MINT_RECEIVER_POLICY, policyId)` if `to` + /// is not authorized under the active `MINT_RECEIVER_POLICY`. + /// 5. `SupplyCapExceeded(cap, totalSupply + amount)` if the + /// mint would push `totalSupply` past `supplyCap`. /// @dev Per-minter rate limiting is NOT enshrined at any level /// (Default or variant). Minter quotas live in EVM /// periphery contracts: a controller / wrapper that holds @@ -578,12 +597,16 @@ interface IB20 { /// after the standard `Transfer` event. function mintWithMemo(address to, uint256 amount, bytes32 memo) external; - /// @notice Burns `amount` from the caller's own balance. Requires - /// `BURN_ROLE`. Subject to `BURN` not being paused (else - /// `ContractPaused(BURN)`). NOT subject to any policy: - /// burn destroys the caller's own supply with no recipient. - /// Reverts with `InsufficientBalance(caller, balance, amount)` - /// if the caller does not have enough balance. + /// @notice Burns `amount` from the caller's own balance. Preconditions + /// evaluate in the order listed; when multiple are violated, + /// the earliest-listed revert is the one that surfaces: + /// 1. `ContractPaused(BURN)` if `BURN` is paused. + /// 2. `AccessControlUnauthorizedAccount(msg.sender, BURN_ROLE)` + /// if the caller does not hold `BURN_ROLE`. + /// 3. `InsufficientBalance(caller, balance, amount)` if the + /// caller does not have enough balance. + /// NOT subject to any policy: burn destroys the caller's own + /// supply with no recipient. /// @dev To destroy balance held by a third party (compliance /// seizure from a policy-blocked address), use `burnBlocked`. /// Emits `Transfer(caller, address(0), amount)`. @@ -593,16 +616,18 @@ interface IB20 { /// after the standard `Transfer` event. function burnWithMemo(uint256 amount, bytes32 memo) external; - /// @notice Destroys `amount` of `from`'s balance. Requires - /// `BURN_BLOCKED_ROLE`. Subject to: - /// 1. `BURN` is not paused (else `ContractPaused(BURN)`). - /// 2. `from` is NOT authorized under the active - /// `TRANSFER_SENDER_POLICY` policy (else `AccountNotBlocked(from)`). - /// `burnBlocked` exists for seizure of policy-blocked - /// balance; calling it against an authorized address is - /// rejected by design. - /// 3. `amount <= balanceOf(from)` (else - /// `InsufficientBalance(from, balance, amount)`). + /// @notice Destroys `amount` of `from`'s balance. Preconditions + /// evaluate in the order listed; when multiple are violated, + /// the earliest-listed revert is the one that surfaces: + /// 1. `ContractPaused(BURN)` if `BURN` is paused. + /// 2. `AccessControlUnauthorizedAccount(msg.sender, BURN_BLOCKED_ROLE)` + /// if the caller does not hold `BURN_BLOCKED_ROLE`. + /// 3. `AccountNotBlocked(from)` if `from` IS authorized under + /// the active `TRANSFER_SENDER_POLICY` policy. `burnBlocked` + /// exists for seizure of policy-blocked balance; calling + /// it against a non-blocked address is rejected by design. + /// 4. `InsufficientBalance(from, balance, amount)` if + /// `amount > balanceOf(from)`. /// @dev Designed for sanctions-seizure flows where compliance /// requires destruction of balance held by a blocked /// address. Tokens that follow a "freeze, never seize" @@ -705,16 +730,22 @@ interface IB20 { /// @notice Pauses the `features` operations. Additive: features /// already paused remain paused, and the listed features /// become paused (duplicates within the call are idempotent). - /// Requires `PAUSE_ROLE`. Reverts with `EmptyFeatureSet` if - /// `features.length == 0`. + /// Reverts in the following canonical order (first violated + /// check wins): + /// 1. `AccessControlUnauthorizedAccount(msg.sender, PAUSE_ROLE)` + /// if the caller does not hold `PAUSE_ROLE`. + /// 2. `EmptyFeatureSet()` if `features.length == 0`. function pause(PausableFeature[] calldata features) external; /// @notice Unpauses the `features` operations. Listed features /// become unpaused; features not listed are unaffected /// (duplicates are idempotent; unpausing a feature that is /// not currently paused is a no-op for that feature). - /// Requires `UNPAUSE_ROLE`. Reverts with `EmptyFeatureSet` - /// if `features.length == 0`. + /// Reverts in the following canonical order (first violated + /// check wins): + /// 1. `AccessControlUnauthorizedAccount(msg.sender, UNPAUSE_ROLE)` + /// if the caller does not hold `UNPAUSE_ROLE`. + /// 2. `EmptyFeatureSet()` if `features.length == 0`. function unpause(PausableFeature[] calldata features) external; /*////////////////////////////////////////////////////////////// diff --git a/src/interfaces/IB20Security.sol b/src/interfaces/IB20Security.sol index 0d4bb8fb..29cb6d9f 100644 --- a/src/interfaces/IB20Security.sol +++ b/src/interfaces/IB20Security.sol @@ -374,18 +374,25 @@ interface IB20Security is IB20 { /// allocations, secondary issuances, etc.) that need to /// land many recipients in one transaction. /// - /// @dev Requires `MINT_ROLE`. Subject to the `MINT_RECEIVER_POLICY` - /// policy per recipient and to the `MINT` pause vector. - /// Reverts with `LengthMismatch(recipients.length, - /// amounts.length)` if the parallel arrays disagree, and - /// with `EmptyBatch()` if either array is empty. - /// All-or-nothing: if any element reverts (e.g. - /// `SupplyCapExceeded` after a partial accumulation, or - /// `PolicyForbids(MINT_RECEIVER_POLICY, ...)` for a - /// policy-blocked recipient), the entire transaction - /// reverts and no partial state is committed. Emits - /// `Transfer(address(0), recipients[i], amounts[i])` per - /// element. Standard usage is to invoke this through + /// @dev Reverts in the following canonical order (first violated + /// check wins): + /// 1. `ContractPaused(MINT)` if `MINT` is paused. + /// 2. `AccessControlUnauthorizedAccount(msg.sender, MINT_ROLE)` + /// if the caller does not hold `MINT_ROLE`. + /// 3. `LengthMismatch(recipients.length, amounts.length)` if + /// the parallel arrays disagree. + /// 4. `EmptyBatch()` if either array is empty. + /// 5..N. Per-element checks inside the loop: `InvalidReceiver`, + /// `PolicyForbids(MINT_RECEIVER_POLICY, ...)`, + /// `SupplyCapExceeded` — see `mint`'s natspec for the + /// per-element precedence. + /// The pause and role gates are evaluated ONCE for the + /// whole batch; per-element gates fire per recipient inside + /// the loop. + /// All-or-nothing: if any element reverts, the entire + /// transaction reverts and no partial state is committed. + /// Emits `Transfer(address(0), recipients[i], amounts[i])` + /// per element. Standard usage is to invoke this through /// `announce(...)`'s `internalCalls`, which brackets the /// issuance with a matching disclosure atomically (see /// the contract-level "Announcement pairing" notes). @@ -410,26 +417,32 @@ interface IB20Security is IB20 { /// debits in one transaction without first arranging for /// each account to be policy-blocked. /// - /// @dev Requires `BURN_FROM_ROLE`. NOT gated by any policy: - /// the corporate-actions desk is trusted to pick the right - /// set of accounts off-chain, and the role grant is the - /// on-chain authorization. Subject to the `BURN` pause - /// vector. Reverts with `LengthMismatch(accounts.length, - /// amounts.length)` if the parallel arrays disagree, and - /// with `EmptyBatch()` if either array is empty. - /// All-or-nothing: if any element reverts (e.g. - /// `InsufficientBalance(accounts[k], balance, amounts[k])`), - /// the entire transaction reverts and no partial state is - /// committed. Emits `Transfer(accounts[i], address(0), - /// amounts[i])` per element; does NOT emit `BurnedBlocked` - /// (that event is reserved for `burnBlocked`'s sanctions - /// semantics). Standard usage is to invoke this through - /// `announce(...)`'s `internalCalls`, which brackets the - /// clawback with a matching disclosure atomically (see the - /// contract-level "Announcement pairing" notes). Direct - /// invocation by a role holder remains permitted for - /// emergency override but produces no `Announcement` / - /// `EndAnnouncement` bracket. + /// @dev `BURN_FROM_ROLE` is enforced WITHOUT the factory-bootstrap + /// bypass — clawback against existing balances has no + /// init-time use case, so the role check is unconditional. + /// NOT gated by any policy: the corporate-actions desk is + /// trusted to pick the right set of accounts off-chain, and + /// the role grant is the on-chain authorization. Reverts in + /// the following canonical order (first violated check wins): + /// 1. `ContractPaused(BURN)` if `BURN` is paused. + /// 2. `AccessControlUnauthorizedAccount(msg.sender, BURN_FROM_ROLE)` + /// if the caller does not hold `BURN_FROM_ROLE`. + /// 3. `LengthMismatch(accounts.length, amounts.length)` if + /// the parallel arrays disagree. + /// 4. `EmptyBatch()` if either array is empty. + /// 5. Per-element `InsufficientBalance(accounts[k], balance, + /// amounts[k])` from `_burnRaw`. + /// All-or-nothing: if any element reverts, the entire + /// transaction reverts and no partial state is committed. + /// Emits `Transfer(accounts[i], address(0), amounts[i])` + /// per element; does NOT emit `BurnedBlocked` (that event + /// is reserved for `burnBlocked`'s sanctions semantics). + /// Standard usage is to invoke this through `announce(...)`'s + /// `internalCalls`, which brackets the clawback with a + /// matching disclosure atomically (see the contract-level + /// "Announcement pairing" notes). Direct invocation by a + /// role holder remains permitted for emergency override but + /// produces no `Announcement` / `EndAnnouncement` bracket. /// /// @param accounts Accounts whose balances will be debited. /// @param amounts Per-account amounts, parallel to `accounts`. @@ -442,15 +455,24 @@ interface IB20Security is IB20 { /// @notice Burns `amount` tokens from the caller, recording intent /// to settle off-chain. /// - /// @dev Subject to the `REDEEM_SENDER_POLICY` policy and to the - /// `REDEEM` pause vector. Reverts with - /// `BelowMinimumRedeemable(shares, minimumRedeemable)` if - /// the corresponding share amount (`amount * - /// sharesToTokensRatio / WAD_PRECISION`) is zero OR is - /// strictly less than `minimumRedeemable`. Zero-share - /// redemptions are always rejected, regardless of - /// `minimumRedeemable`'s configured value, so a holder - /// cannot burn token dust that resolves to no shares. + /// @dev Reverts in the following canonical order (first violated + /// check wins): + /// 1. `ContractPaused(REDEEM)` if `REDEEM` is paused. + /// Enforced WITHOUT the factory-bootstrap bypass — redeem + /// is a holder-initiated path with no legitimate init-time + /// use case. + /// 2. `PolicyForbids(REDEEM_SENDER_POLICY, policyId)` if + /// `msg.sender` is not authorized under the active + /// `REDEEM_SENDER_POLICY` policy. + /// 3. `BelowMinimumRedeemable(shares, minimumRedeemable)` if + /// the corresponding share amount (`amount * + /// sharesToTokensRatio / WAD_PRECISION`) is zero OR is + /// strictly less than `minimumRedeemable`. Zero-share + /// redemptions are always rejected, regardless of + /// `minimumRedeemable`'s configured value, so a holder + /// cannot burn token dust that resolves to no shares. + /// 4. `InsufficientBalance(caller, balance, amount)` if the + /// caller does not have enough balance. /// Emits `Transfer(caller, address(0), amount)` followed by /// `Redeemed(caller, amount, sharesToTokensRatio)`. /// diff --git a/test/lib/mocks/MockB20.sol b/test/lib/mocks/MockB20.sol index 8a149de4..672e1b01 100644 --- a/test/lib/mocks/MockB20.sol +++ b/test/lib/mocks/MockB20.sol @@ -36,7 +36,11 @@ import {B20Constants} from "src/lib/B20Constants.sol"; /// writes `initialized = true` directly (also via `vm.store`) /// once `initCalls` have run, closing the privileged window. /// Token invariants (supply-cap math, balance accounting) -/// are NOT bypassed during the window. +/// are NOT bypassed during the window. Pause is also NOT +/// bypassed: pause defaults to "nothing paused" at creation, +/// and any pause state during bootstrap is explicitly opted +/// into by the operator's initCalls, so start-paused +/// configurations must sequence the `pause(...)` call last. /// - Variant tokens (e.g. `MockB20Stablecoin`) extend by adding /// a disjoint storage namespace; the factory writes the /// variant-specific slots directly, no virtual hook needed. @@ -95,6 +99,24 @@ contract MockB20 is IB20 { // MODIFIERS // ============================================================ + /// @dev Reverts `ContractPaused(feature)` if `feature` is paused. + /// Always listed FIRST in a gated function's modifier set so + /// pause precedence reads off the signature directly (modifiers + /// run left-to-right, body at `_`). + /// + /// Unlike `onlyRole` and the policy checks, this guard does + /// NOT honor the factory bootstrap bypass: pause defaults to + /// "nothing paused" at creation, and any pause state during + /// bootstrap is explicitly opted into by the operator's + /// initCalls. There's no legitimate flow where the factory + /// pauses a feature in initCall N and then needs to use that + /// feature in initCall N+1 — start-paused configurations + /// should sequence the `pause(...)` call last. + modifier whenNotPaused(PausableFeature feature) { + if (_isPaused(feature)) revert ContractPaused(feature); + _; + } + /// @dev Gates a function on `msg.sender` holding `role`. Reverts /// `AccessControlUnauthorizedAccount` if not. The factory /// bootstrap window (`_isPrivileged()`) bypasses the check; @@ -148,12 +170,18 @@ contract MockB20 is IB20 { // ERC-20: MUTATIONS // ============================================================ - function transfer(address to, uint256 amount) external returns (bool) { + function transfer(address to, uint256 amount) external whenNotPaused(PausableFeature.TRANSFER) returns (bool) { + _requireNonZeroActors(msg.sender, to); _transfer(msg.sender, to, amount); return true; } - function transferFrom(address from, address to, uint256 amount) external returns (bool) { + function transferFrom(address from, address to, uint256 amount) + external + whenNotPaused(PausableFeature.TRANSFER) + returns (bool) + { + _requireNonZeroActors(from, to); if (!_isPrivileged()) { // Allowance is consumed unconditionally outside the factory // bootstrap window. Matches OZ ERC20 and the Rust precompile, @@ -186,13 +214,23 @@ contract MockB20 is IB20 { // MEMO TRANSFER VARIANTS // ============================================================ - function transferWithMemo(address to, uint256 amount, bytes32 memo) external returns (bool) { + function transferWithMemo(address to, uint256 amount, bytes32 memo) + external + whenNotPaused(PausableFeature.TRANSFER) + returns (bool) + { + _requireNonZeroActors(msg.sender, to); _transfer(msg.sender, to, amount); emit Memo(msg.sender, memo); return true; } - function transferFromWithMemo(address from, address to, uint256 amount, bytes32 memo) external returns (bool) { + function transferFromWithMemo(address from, address to, uint256 amount, bytes32 memo) + external + whenNotPaused(PausableFeature.TRANSFER) + returns (bool) + { + _requireNonZeroActors(from, to); if (!_isPrivileged()) { _consumeAllowance(from, msg.sender, amount); if (msg.sender != from) { @@ -226,27 +264,40 @@ contract MockB20 is IB20 { // MINT / BURN // ============================================================ - function mint(address to, uint256 amount) external { + function mint(address to, uint256 amount) external whenNotPaused(PausableFeature.MINT) onlyRole(MINT_ROLE) { + if (to == address(0)) revert InvalidReceiver(to); _mint(to, amount); } - function mintWithMemo(address to, uint256 amount, bytes32 memo) external { + function mintWithMemo(address to, uint256 amount, bytes32 memo) + external + whenNotPaused(PausableFeature.MINT) + onlyRole(MINT_ROLE) + { + if (to == address(0)) revert InvalidReceiver(to); _mint(to, amount); emit Memo(msg.sender, memo); } - function burn(uint256 amount) external { - _burnSelf(msg.sender, amount); + function burn(uint256 amount) external whenNotPaused(PausableFeature.BURN) onlyRole(BURN_ROLE) { + _burnRaw(msg.sender, amount); } - function burnWithMemo(uint256 amount, bytes32 memo) external { - _burnSelf(msg.sender, amount); + function burnWithMemo(uint256 amount, bytes32 memo) + external + whenNotPaused(PausableFeature.BURN) + onlyRole(BURN_ROLE) + { + _burnRaw(msg.sender, amount); emit Memo(msg.sender, memo); } - function burnBlocked(address from, uint256 amount) external onlyRole(BURN_BLOCKED_ROLE) { + function burnBlocked(address from, uint256 amount) + external + whenNotPaused(PausableFeature.BURN) + onlyRole(BURN_BLOCKED_ROLE) + { if (!_isPrivileged()) { - if (_isPaused(PausableFeature.BURN)) revert ContractPaused(PausableFeature.BURN); // The point of burnBlocked is to seize from policy-blocked // accounts. Read the transfer-sender policy ID out of the // transfer-side packed slot and reject if the target is @@ -621,12 +672,26 @@ contract MockB20 is IB20 { } } - function _transfer(address from, address to, uint256 amount) internal { + /// @dev Receiver-then-sender zero-address check shared by every + /// transfer-family entrypoint. Reverts `InvalidReceiver(to)` + /// before `InvalidSender(from)` so the precedence between the + /// two matches the canonical order. + function _requireNonZeroActors(address from, address to) internal pure { if (to == address(0)) revert InvalidReceiver(to); if (from == address(0)) revert InvalidSender(from); + } + /// @dev Pure mechanics: policy (with bootstrap bypass) + balance + + /// effects. Pause + input validation are enforced upstream by + /// every external caller (`transfer`, `transferFrom`, + /// `transferWithMemo`, `transferFromWithMemo`) before reaching + /// this helper. `transferFrom` / `transferFromWithMemo` + /// additionally consume allowance and check the executor + /// policy in their bodies before calling here; both of those + /// checks ALSO honor the bootstrap bypass, consistent with + /// the policy bypass below. + function _transfer(address from, address to, uint256 amount) internal { if (!_isPrivileged()) { - if (_isPaused(PausableFeature.TRANSFER)) revert ContractPaused(PausableFeature.TRANSFER); // One SLOAD pulls both policy IDs we need for the transfer // check (and was already warmed if we came in via transferFrom, // which reads the executor lane of the same slot first). @@ -651,11 +716,14 @@ contract MockB20 is IB20 { emit Transfer(from, to, amount); } - function _mint(address to, uint256 amount) internal onlyRole(MINT_ROLE) { - if (to == address(0)) revert InvalidReceiver(to); - + /// @dev Pure mechanics: policy (with bootstrap bypass) + supply cap + /// + effects. Pause, role, and the zero-receiver check are + /// enforced upstream by `mint` / `mintWithMemo`. The security + /// variant's `batchMint` carries the same `whenNotPaused` + + /// `onlyRole` modifiers ONCE for the whole batch and validates + /// per-element receivers inline before invoking this helper. + function _mint(address to, uint256 amount) internal { if (!_isPrivileged()) { - if (_isPaused(PausableFeature.MINT)) revert ContractPaused(PausableFeature.MINT); uint64 mintReceiverPolicyId = MockB20Storage.layout().mintPolicyIds.receiver; if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(mintReceiverPolicyId, to)) { revert PolicyForbids(MINT_RECEIVER_POLICY, mintReceiverPolicyId); @@ -672,13 +740,13 @@ contract MockB20 is IB20 { emit Transfer(address(0), to, amount); } - function _burnSelf(address from, uint256 amount) internal onlyRole(BURN_ROLE) { - if (!_isPrivileged()) { - if (_isPaused(PausableFeature.BURN)) revert ContractPaused(PausableFeature.BURN); - } - _burnRaw(from, amount); - } - + /// @dev Pure mechanics: balance + effects. Pause and role gates are + /// enforced by entrypoint modifiers (`whenNotPaused`, + /// `onlyRole`) on every caller (`burn`, `burnWithMemo`, + /// `burnBlocked`, and `MockB20Security`'s `batchBurn` / + /// `_redeemBurn`); see those functions for the per-caller + /// authorization surface. This helper never authorizes the + /// destruction on its own. function _burnRaw(address from, uint256 amount) internal { MockB20Storage.Layout storage $ = MockB20Storage.layout(); uint256 fromBalance = $.balances[from]; diff --git a/test/lib/mocks/MockB20Security.sol b/test/lib/mocks/MockB20Security.sol index fe5a72ed..81ef02ca 100644 --- a/test/lib/mocks/MockB20Security.sol +++ b/test/lib/mocks/MockB20Security.sol @@ -173,13 +173,19 @@ contract MockB20Security is MockB20, IB20Security { // BATCHED ISSUANCE / CLAWBACK // ============================================================ - function batchMint(address[] calldata recipients, uint256[] calldata amounts) external { + /// @dev Pause + role enforced ONCE for the entire batch via the + /// entrypoint modifiers. Per-element zero-receiver guard is + /// inlined in the loop since `_mint` no longer carries an + /// input check. + function batchMint(address[] calldata recipients, uint256[] calldata amounts) + external + whenNotPaused(PausableFeature.MINT) + onlyRole(MINT_ROLE) + { if (recipients.length != amounts.length) revert LengthMismatch(recipients.length, amounts.length); if (recipients.length == 0) revert EmptyBatch(); - // Per-element call into _mint: role / pause checks repeat (idempotent), - // but MINT_RECEIVER_POLICY policy and supply-cap accumulation are correctly - // applied per recipient. Cleaner than re-deriving the per-element body. for (uint256 i = 0; i < recipients.length; i++) { + if (recipients[i] == address(0)) revert InvalidReceiver(recipients[i]); _mint(recipients[i], amounts[i]); } } @@ -191,13 +197,13 @@ contract MockB20Security is MockB20, IB20Security { /// the attack surface. function batchBurn(address[] calldata accounts, uint256[] calldata amounts) external + whenNotPaused(PausableFeature.BURN) onlyRoleStrict(BURN_FROM_ROLE) { if (accounts.length != amounts.length) { revert LengthMismatch(accounts.length, amounts.length); } if (accounts.length == 0) revert EmptyBatch(); - if (_isPaused(PausableFeature.BURN)) revert ContractPaused(PausableFeature.BURN); for (uint256 i = 0; i < accounts.length; i++) { // Zero amounts are allowed per ERC-20 conventions (`transfer(0)` is valid); // `_burnRaw` is a no-op for amount == 0 and emits `Transfer(account, 0, 0)`. @@ -210,12 +216,12 @@ contract MockB20Security is MockB20, IB20Security { // REDEMPTION // ============================================================ - function redeem(uint256 amount) external { + function redeem(uint256 amount) external whenNotPaused(PausableFeature.REDEEM) { uint256 ratio = _redeemBurn(amount); emit Redeemed(msg.sender, amount, ratio); } - function redeemWithMemo(uint256 amount, bytes32 memo) external { + function redeemWithMemo(uint256 amount, bytes32 memo) external whenNotPaused(PausableFeature.REDEEM) { uint256 ratio = _redeemBurn(amount); // Order matters: Transfer (in _redeemBurn), then Memo, then Redeemed. emit Memo(msg.sender, memo); @@ -289,7 +295,6 @@ contract MockB20Security is MockB20, IB20Security { /// path that the factory has no legitimate reason to invoke /// during bootstrap, so the bypass is omitted by design. function _redeemBurn(uint256 amount) internal returns (uint256 ratio) { - if (_isPaused(PausableFeature.REDEEM)) revert ContractPaused(PausableFeature.REDEEM); MockB20RedeemStorage.Layout storage $ = MockB20RedeemStorage.layout(); uint64 REDEEMSenderPolicyId = $.redeemPolicyIds.sender; if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(REDEEMSenderPolicyId, msg.sender)) { diff --git a/test/unit/B20/erc20/transferFrom_revertOrder.t.sol b/test/unit/B20/erc20/transferFrom_revertOrder.t.sol index d573d003..63cb0231 100644 --- a/test/unit/B20/erc20/transferFrom_revertOrder.t.sol +++ b/test/unit/B20/erc20/transferFrom_revertOrder.t.sol @@ -9,18 +9,138 @@ import {PolicyRegistryConstants} from "test/lib/mocks/MockPolicyRegistry.sol"; /// @title Differential check-order tests for `transferFrom`. /// -/// @notice `transferFrom` adds two preconditions on top of `_transfer`'s checks -/// (already pinned by `transfer_revertOrder.t.sol`): +/// @notice `transferFrom` layers two body-level preconditions +/// (ALLOWANCE and EXECUTOR-POLICY) on top of `_transfer`'s +/// policy / balance checks. The PAUSE / ZERO-RECEIVER / +/// ZERO-SENDER guards run before the allowance / executor-policy +/// work in the entrypoint body. /// -/// **Canonical order (Solidity reference, when `msg.sender != from`):** -/// 1. ALLOWANCE (`_consumeAllowance`) → `InsufficientAllowance` -/// 2. EXECUTOR-POLICY (`isAuthorized(executorPolicyId, msg.sender)`) → `PolicyForbids(EXECUTOR, ...)` -/// 3..N. (all `_transfer` body checks — see `transfer_revertOrder.t.sol`) +/// **Canonical order (Solidity reference, when +/// `msg.sender != from`):** +/// 1. PAUSE (`whenNotPaused(TRANSFER)` modifier) → `ContractPaused` +/// 2. ZERO-RECEIVER (`to == address(0)`) → `InvalidReceiver` +/// 3. ZERO-SENDER (`from == address(0)`) → `InvalidSender` +/// 4. ALLOWANCE (`_consumeAllowance`) → `InsufficientAllowance` +/// 5. EXECUTOR-POLICY (`isAuthorized(executorPolicyId, msg.sender)`) +/// → `PolicyForbids(EXECUTOR, ...)` +/// 6..N. All `_transfer` body checks — see `transfer_revertOrder.t.sol` +/// (SENDER-POLICY → RECEIVER-POLICY → BALANCE). /// -/// These tests pin the two new preconditions against each other and -/// against a representative `_transfer` body check (ZERO-RECEIVER) to -/// prove they fire before `_transfer` is entered. +/// The full pair matrix between body-level ALLOWANCE/EXECUTOR-POLICY +/// and the PAUSE/ZERO-RECEIVER/ZERO-SENDER guards is pinned below; +/// one test against a representative `_transfer` body check +/// (SENDER-POLICY) proves ALLOWANCE and EXECUTOR-POLICY both +/// fire before `_transfer` is entered. contract B20TransferFromRevertOrderTest is B20Test { + // --- Pairs where PAUSE wins (PAUSE is canonical first) --- + + /// @notice PAUSE beats ALLOWANCE. + function test_transferFrom_revertOrder_pause_beats_allowance( + address caller, + address from, + address to, + uint256 amount + ) public { + _assumeValidCaller(caller); + _assumeValidActor(from); + _assumeValidActor(to); + vm.assume(caller != from); + amount = bound(amount, 1, type(uint128).max); + _pause(IB20.PausableFeature.TRANSFER); + // No allowance set → ALLOWANCE would fire if PAUSE didn't. + + vm.prank(caller); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.TRANSFER)); + token.transferFrom(from, to, amount); + } + + /// @notice PAUSE beats EXECUTOR-POLICY. + function test_transferFrom_revertOrder_pause_beats_executorPolicy( + address caller, + address from, + address to, + uint256 amount + ) public { + _assumeValidCaller(caller); + _assumeValidActor(from); + _assumeValidActor(to); + vm.assume(caller != from); + amount = bound(amount, 1, type(uint128).max); + _pause(IB20.PausableFeature.TRANSFER); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(caller); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.TRANSFER)); + token.transferFrom(from, to, amount); + } + + // --- Pairs where ZERO-RECEIVER wins (PAUSE not violated) --- + + /// @notice ZERO-RECEIVER beats ALLOWANCE. + /// @dev Zero-receiver check fires before the allowance check. + function test_transferFrom_revertOrder_zeroReceiver_beats_allowance(address caller, address from, uint256 amount) + public + { + _assumeValidCaller(caller); + _assumeValidActor(from); + vm.assume(caller != from); + amount = bound(amount, 1, type(uint128).max); + // No allowance AND `to == address(0)`. + + vm.prank(caller); + vm.expectRevert(abi.encodeWithSelector(IB20.InvalidReceiver.selector, address(0))); + token.transferFrom(from, address(0), amount); + } + + /// @notice ZERO-RECEIVER beats EXECUTOR-POLICY. + function test_transferFrom_revertOrder_zeroReceiver_beats_executorPolicy( + address caller, + address from, + uint256 amount + ) public { + _assumeValidCaller(caller); + _assumeValidActor(from); + vm.assume(caller != from); + amount = bound(amount, 1, type(uint128).max); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(caller); + vm.expectRevert(abi.encodeWithSelector(IB20.InvalidReceiver.selector, address(0))); + token.transferFrom(from, address(0), amount); + } + + // --- Pairs where ZERO-SENDER wins (PAUSE + ZERO-RECEIVER not violated) --- + + /// @notice ZERO-SENDER beats ALLOWANCE. + /// @dev Zero-sender check fires before the allowance check. + function test_transferFrom_revertOrder_zeroSender_beats_allowance(address caller, address to, uint256 amount) + public + { + _assumeValidCaller(caller); + _assumeValidActor(to); + amount = bound(amount, 1, type(uint128).max); + + vm.prank(caller); + vm.expectRevert(abi.encodeWithSelector(IB20.InvalidSender.selector, address(0))); + token.transferFrom(address(0), to, amount); + } + + /// @notice ZERO-SENDER beats EXECUTOR-POLICY. + function test_transferFrom_revertOrder_zeroSender_beats_executorPolicy(address caller, address to, uint256 amount) + public + { + _assumeValidCaller(caller); + _assumeValidActor(to); + amount = bound(amount, 1, type(uint128).max); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(caller); + vm.expectRevert(abi.encodeWithSelector(IB20.InvalidSender.selector, address(0))); + token.transferFrom(address(0), to, amount); + } + + // --- Pairs where ALLOWANCE wins (PAUSE + input not violated) --- + /// @notice ALLOWANCE beats EXECUTOR-POLICY. function test_transferFrom_revertOrder_allowance_beats_executorPolicy( address caller, @@ -41,37 +161,51 @@ contract B20TransferFromRevertOrderTest is B20Test { token.transferFrom(from, to, amount); } - /// @notice ALLOWANCE beats anything in `_transfer` (representative: ZERO-RECEIVER). - function test_transferFrom_revertOrder_allowance_beats_transferBody(address caller, address from, uint256 amount) - public - { + /// @notice ALLOWANCE beats anything in `_transfer` (representative: SENDER-POLICY). + /// @dev `_transfer`'s body is policy + balance + effects (input validation + /// lives on the entrypoint); SENDER-POLICY is the first body check + /// inside `_transfer`. + function test_transferFrom_revertOrder_allowance_beats_transferBody( + address caller, + address from, + address to, + uint256 amount + ) public { _assumeValidCaller(caller); _assumeValidActor(from); + _assumeValidActor(to); vm.assume(caller != from); amount = bound(amount, 1, type(uint128).max); - // Allowance is 0 AND `to` is address(0). + // Allowance is 0 AND sender policy blocks `from`. + _setPolicy(B20Constants.TRANSFER_SENDER_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); vm.prank(caller); vm.expectRevert(abi.encodeWithSelector(IB20.InsufficientAllowance.selector, caller, 0, amount)); - token.transferFrom(from, address(0), amount); + token.transferFrom(from, to, amount); } - /// @notice EXECUTOR-POLICY beats anything in `_transfer` (representative: ZERO-RECEIVER). - /// @dev Allowance is set high enough to pass the allowance check, so the executor-policy - /// check is reached next, and it fires before `_transfer` is entered. + // --- Pair where EXECUTOR-POLICY wins (everything earlier satisfied) --- + + /// @notice EXECUTOR-POLICY beats anything in `_transfer` (representative: SENDER-POLICY). + /// @dev Allowance is set high enough to pass the allowance check, so the + /// executor-policy check runs next and fires before `_transfer` is + /// entered. function test_transferFrom_revertOrder_executorPolicy_beats_transferBody( address caller, address from, + address to, uint256 amount ) public { _assumeValidCaller(caller); _assumeValidActor(from); + _assumeValidActor(to); vm.assume(caller != from); amount = bound(amount, 1, type(uint128).max); - // Sufficient allowance, executor policy blocks caller, `to` is address(0). + // Sufficient allowance, executor policy blocks caller, sender policy also blocks `from`. vm.prank(from); token.approve(caller, amount); _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + _setPolicy(B20Constants.TRANSFER_SENDER_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); vm.prank(caller); vm.expectRevert( @@ -81,6 +215,6 @@ contract B20TransferFromRevertOrderTest is B20Test { PolicyRegistryConstants.ALWAYS_BLOCK_ID ) ); - token.transferFrom(from, address(0), amount); + token.transferFrom(from, to, amount); } } diff --git a/test/unit/B20/erc20/transfer_revertOrder.t.sol b/test/unit/B20/erc20/transfer_revertOrder.t.sol index f1a7313f..af36a778 100644 --- a/test/unit/B20/erc20/transfer_revertOrder.t.sol +++ b/test/unit/B20/erc20/transfer_revertOrder.t.sol @@ -9,31 +9,42 @@ import {PolicyRegistryConstants} from "test/lib/mocks/MockPolicyRegistry.sol"; /// @title Differential check-order tests for `transfer`. /// -/// @notice **Canonical order (Solidity reference `_transfer`):** -/// 1. ZERO-RECEIVER (`to == address(0)`) → `InvalidReceiver` -/// 2. ZERO-SENDER (`from == address(0)`) → `InvalidSender` -/// 3. PAUSE (`_isPaused(TRANSFER)`) → `ContractPaused` -/// 4. SENDER-POLICY (`isAuthorized(senderPolicyId, from)`) → `PolicyForbids(SENDER, ...)` -/// 5. RECEIVER-POLICY (`isAuthorized(receiverPolicyId, to)`) → `PolicyForbids(RECEIVER, ...)` -/// 6. BALANCE (`fromBalance < amount`) → `InsufficientBalance` +/// @notice **Canonical order (Solidity reference):** +/// 1. PAUSE (`whenNotPaused(TRANSFER)` modifier) → `ContractPaused` +/// 2. ZERO-RECEIVER (`to == address(0)`) → `InvalidReceiver` +/// 3. ZERO-SENDER (`from == address(0)`) → `InvalidSender` +/// 4. SENDER-POLICY (`_transfer` body) → `PolicyForbids(SENDER, ...)` +/// 5. RECEIVER-POLICY (`_transfer` body) → `PolicyForbids(RECEIVER, ...)` +/// 6. BALANCE (`_transfer` body) → `InsufficientBalance` /// /// The public `transfer(to, amount)` entry sets `from = msg.sender`, so /// pairs involving ZERO-SENDER require pranking `address(0)`. C(6, 2) = 15 pairs. contract B20TransferRevertOrderTest is B20Test { - // --- Pairs where ZERO-RECEIVER wins --- + // --- Pairs where PAUSE wins (PAUSE is canonical first) --- - function test_transfer_revertOrder_zeroReceiver_beats_zeroSender(uint256 amount) public { - // Both `to` and `from` are address(0) — receiver check fires first. - vm.prank(address(0)); - vm.expectRevert(abi.encodeWithSelector(IB20.InvalidReceiver.selector, address(0))); + function test_transfer_revertOrder_pause_beats_zeroReceiver(address from, uint256 amount) public { + _assumeValidActor(from); + _pause(IB20.PausableFeature.TRANSFER); + + vm.prank(from); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.TRANSFER)); token.transfer(address(0), amount); } - function test_transfer_revertOrder_zeroReceiver_beats_pause(address from, uint256 amount) public { - _assumeValidActor(from); + function test_transfer_revertOrder_pause_beats_zeroSender(address to, uint256 amount) public { + _assumeValidActor(to); _pause(IB20.PausableFeature.TRANSFER); - vm.prank(from); + vm.prank(address(0)); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.TRANSFER)); + token.transfer(to, amount); + } + + // --- Pairs where ZERO-RECEIVER wins (PAUSE not violated) --- + + function test_transfer_revertOrder_zeroReceiver_beats_zeroSender(uint256 amount) public { + // Both `to` and `from` are address(0) — receiver check fires first. + vm.prank(address(0)); vm.expectRevert(abi.encodeWithSelector(IB20.InvalidReceiver.selector, address(0))); token.transfer(address(0), amount); } @@ -66,16 +77,7 @@ contract B20TransferRevertOrderTest is B20Test { token.transfer(address(0), amount); } - // --- Pairs where ZERO-SENDER wins (requires pranking address(0)) --- - - function test_transfer_revertOrder_zeroSender_beats_pause(address to, uint256 amount) public { - _assumeValidActor(to); - _pause(IB20.PausableFeature.TRANSFER); - - vm.prank(address(0)); - vm.expectRevert(abi.encodeWithSelector(IB20.InvalidSender.selector, address(0))); - token.transfer(to, amount); - } + // --- Pairs where ZERO-SENDER wins (PAUSE not violated; requires pranking address(0)) --- function test_transfer_revertOrder_zeroSender_beats_senderPolicy(address to, uint256 amount) public { _assumeValidActor(to); diff --git a/test/unit/B20/supply/burnBlocked_revertOrder.t.sol b/test/unit/B20/supply/burnBlocked_revertOrder.t.sol index c1895e4f..94f572c3 100644 --- a/test/unit/B20/supply/burnBlocked_revertOrder.t.sol +++ b/test/unit/B20/supply/burnBlocked_revertOrder.t.sol @@ -10,8 +10,8 @@ import {PolicyRegistryConstants} from "test/lib/mocks/MockPolicyRegistry.sol"; /// @title Differential check-order tests for `burnBlocked`. /// /// @notice **Canonical order (Solidity reference):** -/// 1. ROLE (`onlyRole(BURN_BLOCKED_ROLE)` modifier) → `AccessControlUnauthorizedAccount` -/// 2. PAUSE (`_isPaused(BURN)`) → `ContractPaused` +/// 1. PAUSE (`whenNotPaused(BURN)` modifier) → `ContractPaused` +/// 2. ROLE (`onlyRole(BURN_BLOCKED_ROLE)` modifier) → `AccessControlUnauthorizedAccount` /// 3. BLOCKED (`isAuthorized(senderPolicyId, from) == true` reverts) → `AccountNotBlocked` /// 4. BALANCE (`fromBalance < amount` in `_burnRaw`) → `InsufficientBalance` /// @@ -21,25 +21,24 @@ import {PolicyRegistryConstants} from "test/lib/mocks/MockPolicyRegistry.sol"; /// violation by setting the policy to `ALWAYS_ALLOW` so every account is /// authorized, which makes the function revert `AccountNotBlocked`. contract B20BurnBlockedRevertOrderTest is B20Test { - // --- Pairs where ROLE wins --- + // --- Pair where PAUSE wins (PAUSE is canonical first) --- - /// @notice ROLE beats PAUSE. - function test_burnBlocked_revertOrder_role_beats_pause(address caller, address from, uint256 amount) public { + /// @notice PAUSE beats ROLE. + /// @dev Pause modifier is listed before the role modifier; fires first. + function test_burnBlocked_revertOrder_pause_beats_role(address caller, address from, uint256 amount) public { _assumeValidCaller(caller); _assumeValidActor(from); vm.assume(caller != admin); _pause(IB20.PausableFeature.BURN); - // No BURN_BLOCKED_ROLE granted. + // No BURN_BLOCKED_ROLE granted AND BURN is paused — pause fires first. vm.prank(caller); - vm.expectRevert( - abi.encodeWithSelector( - IB20.AccessControlUnauthorizedAccount.selector, caller, B20Constants.BURN_BLOCKED_ROLE - ) - ); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); token.burnBlocked(from, amount); } + // --- Pairs where ROLE wins (PAUSE not violated) --- + /// @notice ROLE beats BLOCKED. function test_burnBlocked_revertOrder_role_beats_blocked(address caller, address from, uint256 amount) public { _assumeValidCaller(caller); diff --git a/test/unit/B20/supply/burn_revertOrder.t.sol b/test/unit/B20/supply/burn_revertOrder.t.sol index 79a8b764..ee5e786d 100644 --- a/test/unit/B20/supply/burn_revertOrder.t.sol +++ b/test/unit/B20/supply/burn_revertOrder.t.sol @@ -12,32 +12,31 @@ import {MockB20, B20Constants} from "test/lib/mocks/MockB20.sol"; /// canonical first-firing revert selector. See `mint_revertOrder.t.sol` /// for the harness rationale. /// -/// **Canonical order (Solidity reference, `_burnSelf` → `_burnRaw`):** -/// 1. ROLE (`onlyRole(BURN_ROLE)` modifier) → `AccessControlUnauthorizedAccount` -/// 2. PAUSE (`_isPaused(BURN)`) → `ContractPaused` +/// **Canonical order (Solidity reference):** +/// 1. PAUSE (`whenNotPaused(BURN)` modifier) → `ContractPaused` +/// 2. ROLE (`onlyRole(BURN_ROLE)` modifier) → `AccessControlUnauthorizedAccount` /// 3. BALANCE (`fromBalance < amount` in `_burnRaw`) → `InsufficientBalance` contract B20BurnRevertOrderTest is B20Test { - /// @notice ROLE beats PAUSE. - /// @dev Modifier runs before any body check, including the pause guard. - function test_burn_revertOrder_role_beats_pause(address caller, uint256 amount) public { + /// @notice PAUSE beats ROLE. + /// @dev Pause modifier is listed before the role modifier; fires first. + function test_burn_revertOrder_pause_beats_role(address caller, uint256 amount) public { _assumeValidCaller(caller); vm.assume(caller != admin); _pause(IB20.PausableFeature.BURN); + // Caller has no role AND BURN is paused — pause fires first. vm.prank(caller); - vm.expectRevert( - abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, B20Constants.BURN_ROLE) - ); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); token.burn(amount); } /// @notice ROLE beats BALANCE. - /// @dev Modifier runs before `_burnRaw` is reached; insufficient balance never gets checked. + /// @dev `onlyRole` modifier runs before `_burnRaw` is reached; insufficient balance never gets checked. function test_burn_revertOrder_role_beats_balance(address caller, uint256 amount) public { _assumeValidCaller(caller); vm.assume(caller != admin); amount = bound(amount, 1, type(uint128).max); - // Caller has zero balance and no role — role check fires first. + // Caller has zero balance and no role — role check fires first (pause not set). vm.prank(caller); vm.expectRevert( @@ -47,7 +46,7 @@ contract B20BurnRevertOrderTest is B20Test { } /// @notice PAUSE beats BALANCE. - /// @dev Pause guard in `_burnSelf` runs before `_burnRaw` is invoked. + /// @dev `whenNotPaused` modifier on the entrypoint fires before `_burnRaw` is invoked. function test_burn_revertOrder_pause_beats_balance(uint256 amount) public { amount = bound(amount, 1, type(uint128).max); _grantRole(B20Constants.BURN_ROLE, alice); diff --git a/test/unit/B20/supply/mint_revertOrder.t.sol b/test/unit/B20/supply/mint_revertOrder.t.sol index b42feccf..fa75e330 100644 --- a/test/unit/B20/supply/mint_revertOrder.t.sol +++ b/test/unit/B20/supply/mint_revertOrder.t.sol @@ -16,51 +16,88 @@ import {MockPolicyRegistry, PolicyRegistryConstants} from "test/lib/mocks/MockPo /// between the two backends surfaces as a fork-mode-only failure with a /// clear "selector A vs selector B" diff. /// -/// **Canonical order (Option A — Solidity's defensive-depth order):** -/// 1. ROLE (`onlyRole(MINT_ROLE)` modifier) → `AccessControlUnauthorizedAccount` -/// 2. ZERO-RECEIVER (`to == address(0)`) → `InvalidReceiver` -/// 3. PAUSE (`_isPaused(MINT)`) → `ContractPaused` -/// 4. POLICY (`isAuthorized(mintReceiverPolicyId, to)`) → `PolicyForbids` -/// 5. SUPPLY-CAP (`totalSupply + amount > supplyCap`) → `SupplyCapExceeded` +/// **Canonical order (Solidity reference):** +/// 1. PAUSE (`whenNotPaused(MINT)` modifier) → `ContractPaused` +/// 2. ROLE (`onlyRole(MINT_ROLE)` modifier) → `AccessControlUnauthorizedAccount` +/// 3. ZERO-RECEIVER (`to == address(0)`) → `InvalidReceiver` +/// 4. POLICY (`_mint` body) → `PolicyForbids` +/// 5. SUPPLY-CAP (`_mint` body) → `SupplyCapExceeded` /// /// A `mint` call that violates two or more preconditions must always /// revert with the selector for the earliest-listed violation. The 10 /// tests below enumerate every pair (C(5, 2) = 10). contract B20MintRevertOrderTest is B20Test { - // --- Pairs where ROLE wins (ROLE is canonical first) --- + // --- Pairs where PAUSE wins (PAUSE is canonical first) --- - /// @notice With both ROLE and ZERO-RECEIVER violated, ROLE fires first. - /// @dev Canonical: the `onlyRole` modifier runs before the `to == 0` body check. - function test_mint_revertOrder_role_beats_zeroRecipient(address caller, uint256 amount) public { + /// @notice With both PAUSE and ROLE violated, PAUSE fires first. + /// @dev Pause modifier is listed before the role modifier; runs first. + function test_mint_revertOrder_pause_beats_role(address caller, address to, uint256 amount) public { _assumeValidCaller(caller); - vm.assume(caller != admin); // admin holds DEFAULT_ADMIN_ROLE but not MINT_ROLE on fresh token - // No MINT_ROLE granted; recipient is address(0). + _assumeValidActor(to); + vm.assume(caller != admin); + _pause(IB20.PausableFeature.MINT); + // No MINT_ROLE granted AND MINT is paused — pause fires first. vm.prank(caller); - vm.expectRevert( - abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, B20Constants.MINT_ROLE) - ); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.MINT)); + token.mint(to, amount); + } + + /// @notice With both PAUSE and ZERO-RECEIVER violated, PAUSE fires first. + /// @dev Pause modifier runs before the body's zero-receiver check. + function test_mint_revertOrder_pause_beats_zeroRecipient(uint256 amount) public { + _grantRole(B20Constants.MINT_ROLE, minter); + _pause(IB20.PausableFeature.MINT); + // minter has role; recipient is address(0); MINT is paused — pause fires first. + + vm.prank(minter); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.MINT)); token.mint(address(0), amount); } - /// @notice With both ROLE and PAUSE violated, ROLE fires first. - /// @dev Canonical: modifier runs before any body check, including the pause guard. - function test_mint_revertOrder_role_beats_pause(address caller, address to, uint256 amount) public { - _assumeValidCaller(caller); + /// @notice With PAUSE and POLICY violated, PAUSE fires first. + function test_mint_revertOrder_pause_beats_policy(address to, uint256 amount) public { _assumeValidActor(to); - vm.assume(caller != admin); + _grantRole(B20Constants.MINT_ROLE, minter); _pause(IB20.PausableFeature.MINT); - // No MINT_ROLE granted. + _setPolicy(B20Constants.MINT_RECEIVER_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(minter); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.MINT)); + token.mint(to, amount); + } + + /// @notice With PAUSE and CAP violated, PAUSE fires first. + function test_mint_revertOrder_pause_beats_cap(address to, uint256 amount) public { + _assumeValidActor(to); + _grantRole(B20Constants.MINT_ROLE, minter); + _pause(IB20.PausableFeature.MINT); + amount = bound(amount, 1, type(uint128).max); + vm.prank(admin); + token.updateSupplyCap(0); + + vm.prank(minter); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.MINT)); + token.mint(to, amount); + } + + // --- Pairs where ROLE wins (PAUSE not violated) --- + + /// @notice With both ROLE and ZERO-RECEIVER violated, ROLE fires first. + /// @dev Role modifier runs before the body's zero-receiver check. + function test_mint_revertOrder_role_beats_zeroRecipient(address caller, uint256 amount) public { + _assumeValidCaller(caller); + vm.assume(caller != admin); // admin holds DEFAULT_ADMIN_ROLE but not MINT_ROLE on fresh token + // No MINT_ROLE granted; recipient is address(0); pause not set. vm.prank(caller); vm.expectRevert( abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, B20Constants.MINT_ROLE) ); - token.mint(to, amount); + token.mint(address(0), amount); } /// @notice With both ROLE and POLICY violated, ROLE fires first. - /// @dev Canonical: modifier runs before the receiver-policy check. function test_mint_revertOrder_role_beats_policy(address caller, address to, uint256 amount) public { _assumeValidCaller(caller); _assumeValidActor(to); @@ -76,7 +113,6 @@ contract B20MintRevertOrderTest is B20Test { } /// @notice With both ROLE and CAP violated, ROLE fires first. - /// @dev Canonical: modifier runs before the supply-cap arithmetic check. function test_mint_revertOrder_role_beats_cap(address caller, address to, uint256 amount) public { _assumeValidCaller(caller); _assumeValidActor(to); @@ -93,21 +129,10 @@ contract B20MintRevertOrderTest is B20Test { token.mint(to, amount); } - // --- Pairs where ZERO-RECEIVER wins (ROLE satisfied) --- - - /// @notice With ZERO-RECEIVER and PAUSE violated, ZERO-RECEIVER fires first. - /// @dev Canonical: zero-receiver guard runs before the pause guard inside `_mint`. - function test_mint_revertOrder_zeroRecipient_beats_pause(uint256 amount) public { - _grantRole(B20Constants.MINT_ROLE, minter); - _pause(IB20.PausableFeature.MINT); - - vm.prank(minter); - vm.expectRevert(abi.encodeWithSelector(IB20.InvalidReceiver.selector, address(0))); - token.mint(address(0), amount); - } + // --- Pairs where ZERO-RECEIVER wins (PAUSE + ROLE satisfied) --- /// @notice With ZERO-RECEIVER and POLICY violated, ZERO-RECEIVER fires first. - /// @dev Canonical: zero-receiver guard runs before the receiver-policy check. + /// @dev Zero-receiver check runs before the receiver-policy check in `_mint`. function test_mint_revertOrder_zeroRecipient_beats_policy(uint256 amount) public { _grantRole(B20Constants.MINT_ROLE, minter); _setPolicy(B20Constants.MINT_RECEIVER_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); @@ -118,7 +143,7 @@ contract B20MintRevertOrderTest is B20Test { } /// @notice With ZERO-RECEIVER and CAP violated, ZERO-RECEIVER fires first. - /// @dev Canonical: zero-receiver guard runs before the supply-cap arithmetic. + /// @dev Zero-receiver check runs before the supply-cap arithmetic in `_mint`. function test_mint_revertOrder_zeroRecipient_beats_cap(uint256 amount) public { _grantRole(B20Constants.MINT_ROLE, minter); amount = bound(amount, 1, type(uint128).max); @@ -130,40 +155,10 @@ contract B20MintRevertOrderTest is B20Test { token.mint(address(0), amount); } - // --- Pairs where PAUSE wins (ROLE + ZERO-RECEIVER satisfied) --- - - /// @notice With PAUSE and POLICY violated, PAUSE fires first. - /// @dev Canonical: pause guard runs before the receiver-policy check. - function test_mint_revertOrder_pause_beats_policy(address to, uint256 amount) public { - _assumeValidActor(to); - _grantRole(B20Constants.MINT_ROLE, minter); - _pause(IB20.PausableFeature.MINT); - _setPolicy(B20Constants.MINT_RECEIVER_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); - - vm.prank(minter); - vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.MINT)); - token.mint(to, amount); - } - - /// @notice With PAUSE and CAP violated, PAUSE fires first. - /// @dev Canonical: pause guard runs before the supply-cap arithmetic. - function test_mint_revertOrder_pause_beats_cap(address to, uint256 amount) public { - _assumeValidActor(to); - _grantRole(B20Constants.MINT_ROLE, minter); - _pause(IB20.PausableFeature.MINT); - amount = bound(amount, 1, type(uint128).max); - vm.prank(admin); - token.updateSupplyCap(0); - - vm.prank(minter); - vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.MINT)); - token.mint(to, amount); - } - - // --- Pair where POLICY wins (ROLE + ZERO + PAUSE satisfied) --- + // --- Pair where POLICY wins (PAUSE + ROLE + ZERO satisfied) --- /// @notice With POLICY and CAP violated, POLICY fires first. - /// @dev Canonical: receiver-policy check runs before the supply-cap arithmetic. + /// @dev Receiver-policy check runs before the supply-cap arithmetic in `_mint`. function test_mint_revertOrder_policy_beats_cap(address to, uint256 amount) public { _assumeValidActor(to); _grantRole(B20Constants.MINT_ROLE, minter); diff --git a/test/unit/B20Security/batch/batchBurn_revertOrder.t.sol b/test/unit/B20Security/batch/batchBurn_revertOrder.t.sol index ea3f9624..e0ff9126 100644 --- a/test/unit/B20Security/batch/batchBurn_revertOrder.t.sol +++ b/test/unit/B20Security/batch/batchBurn_revertOrder.t.sol @@ -9,10 +9,10 @@ import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; /// @title Differential check-order tests for `batchBurn` (security variant). /// /// @notice **Canonical order (Solidity reference):** -/// 1. ROLE (`onlyRoleStrict(BURN_FROM_ROLE)` modifier) → `AccessControlUnauthorizedAccount` -/// 2. LENGTH_MISMATCH (`accounts.length != amounts.length`) → `LengthMismatch` -/// 3. EMPTY_BATCH (`accounts.length == 0`) → `EmptyBatch` -/// 4. PAUSE (`_isPaused(BURN)`) → `ContractPaused` +/// 1. PAUSE (`whenNotPaused(BURN)` modifier) → `ContractPaused` +/// 2. ROLE (`onlyRoleStrict(BURN_FROM_ROLE)` modifier) → `AccessControlUnauthorizedAccount` +/// 3. LENGTH_MISMATCH (`accounts.length != amounts.length`) → `LengthMismatch` +/// 4. EMPTY_BATCH (`accounts.length == 0`) → `EmptyBatch` /// 5. BALANCE (per-element `_burnRaw`) → `InsufficientBalance` /// /// Some pairs are not reachable because the violations are mutually @@ -33,7 +33,54 @@ contract B20SecurityBatchBurnRevertOrderTest is B20SecurityTest { _twoUints.push(1); } - // --- Pairs where ROLE wins (via modifier, before any body check) --- + // --- Pairs where PAUSE wins (PAUSE is canonical first) --- + + /// @notice PAUSE beats ROLE. + /// @dev Pause modifier is listed before the role-strict modifier; fires first. + function test_batchBurn_revertOrder_pause_beats_role(address caller, uint256 amount) public { + _assumeValidCaller(caller); + vm.assume(caller != admin); + vm.assume(caller != burnFromActor); + _pause(IB20.PausableFeature.BURN); + + vm.prank(caller); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); + security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); + } + + /// @notice PAUSE beats LENGTH_MISMATCH. + function test_batchBurn_revertOrder_pause_beats_lengthMismatch() public { + _grantBurnFrom(); + _pause(IB20.PausableFeature.BURN); + + vm.prank(burnFromActor); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); + security().batchBurn(_twoAddrs, _twoUints); + } + + /// @notice PAUSE beats EMPTY_BATCH. + function test_batchBurn_revertOrder_pause_beats_emptyBatch() public { + _grantBurnFrom(); + _pause(IB20.PausableFeature.BURN); + + vm.prank(burnFromActor); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); + security().batchBurn(_emptyAddrs, _emptyUints); + } + + /// @notice PAUSE beats BALANCE. + function test_batchBurn_revertOrder_pause_beats_balance(uint256 amount) public { + amount = bound(amount, 1, type(uint128).max); + _grantBurnFrom(); + _pause(IB20.PausableFeature.BURN); + // alice has zero balance → BALANCE would fire if PAUSE didn't. + + vm.prank(burnFromActor); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); + security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); + } + + // --- Pairs where ROLE wins (PAUSE not violated) --- // // Note: BURN_FROM_ROLE() is resolved before vm.prank because the view call // would otherwise consume the prank intended for batchBurn (same pattern as @@ -62,18 +109,6 @@ contract B20SecurityBatchBurnRevertOrderTest is B20SecurityTest { security().batchBurn(_emptyAddrs, _emptyUints); } - function test_batchBurn_revertOrder_role_beats_pause(address caller, uint256 amount) public { - _assumeValidCaller(caller); - vm.assume(caller != admin); - vm.assume(caller != burnFromActor); - _pause(IB20.PausableFeature.BURN); - bytes32 role = security().BURN_FROM_ROLE(); - - vm.prank(caller); - vm.expectRevert(abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, role)); - security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); - } - function test_batchBurn_revertOrder_role_beats_balance(address caller, uint256 amount) public { _assumeValidCaller(caller); vm.assume(caller != admin); @@ -86,39 +121,4 @@ contract B20SecurityBatchBurnRevertOrderTest is B20SecurityTest { vm.expectRevert(abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, role)); security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); } - - // --- Pairs where LENGTH_MISMATCH wins --- - - function test_batchBurn_revertOrder_lengthMismatch_beats_pause() public { - _grantBurnFrom(); - _pause(IB20.PausableFeature.BURN); - - vm.prank(burnFromActor); - vm.expectRevert(abi.encodeWithSelector(IB20Security.LengthMismatch.selector, uint256(2), uint256(1))); - security().batchBurn(_twoAddrs, _twoUints); - } - - // --- Pairs where EMPTY_BATCH wins --- - - function test_batchBurn_revertOrder_emptyBatch_beats_pause() public { - _grantBurnFrom(); - _pause(IB20.PausableFeature.BURN); - - vm.prank(burnFromActor); - vm.expectRevert(IB20Security.EmptyBatch.selector); - security().batchBurn(_emptyAddrs, _emptyUints); - } - - // --- Pair where PAUSE wins --- - - function test_batchBurn_revertOrder_pause_beats_balance(uint256 amount) public { - amount = bound(amount, 1, type(uint128).max); - _grantBurnFrom(); - _pause(IB20.PausableFeature.BURN); - // alice has zero balance → BALANCE would fire if PAUSE didn't. - - vm.prank(burnFromActor); - vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); - security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); - } } diff --git a/test/unit/B20Security/batch/batchMint.t.sol b/test/unit/B20Security/batch/batchMint.t.sol index 561e358d..49e08f38 100644 --- a/test/unit/B20Security/batch/batchMint.t.sol +++ b/test/unit/B20Security/batch/batchMint.t.sol @@ -11,29 +11,38 @@ import {MockPolicyRegistry, PolicyRegistryConstants} from "test/lib/mocks/MockPo contract B20SecurityBatchMintTest is B20SecurityTest { /// @notice Verifies batchMint reverts when recipients.length != amounts.length - /// @dev Length-mismatch guard fires before the empty-batch guard; checks - /// LengthMismatch(recipients.length, amounts.length). + /// @dev Length-mismatch guard fires in batchMint's body, after the entrypoint + /// modifiers (PAUSE + MINT_ROLE) and before the empty-batch guard. + /// Caller is granted MINT_ROLE so the role modifier passes and the body + /// reaches the length check. function test_batchMint_revert_lengthMismatch() public { + _grantRole(B20Constants.MINT_ROLE, minter); + address[] memory recipients = _singletonAddresses(alice); uint256[] memory amounts = new uint256[](2); amounts[0] = 1; amounts[1] = 2; + vm.prank(minter); vm.expectRevert(abi.encodeWithSelector(IB20Security.LengthMismatch.selector, uint256(1), uint256(2))); security().batchMint(recipients, amounts); } /// @notice Verifies batchMint reverts when both arrays are empty - /// @dev EmptyBatch guard: no-op corp-actions transactions are rejected to keep the - /// log stream meaningful. + /// @dev EmptyBatch guard fires in batchMint's body, after PAUSE + MINT_ROLE + /// modifiers. Caller is granted MINT_ROLE so the role modifier passes. function test_batchMint_revert_emptyBatch() public { + _grantRole(B20Constants.MINT_ROLE, minter); + + vm.prank(minter); vm.expectRevert(IB20Security.EmptyBatch.selector); security().batchMint(new address[](0), new uint256[](0)); } - /// @notice Verifies batchMint surfaces _mint's role-check revert when caller lacks MINT_ROLE - /// @dev batchMint has no outer role check; per-element `_mint` enforces MINT_ROLE. Any - /// non-minter caller hits the inner revert on element 0. + /// @notice Verifies batchMint reverts when caller lacks MINT_ROLE + /// @dev `onlyRole(MINT_ROLE)` is now an entrypoint modifier on batchMint + /// itself (was per-element via `_mint` before the pause→role→input + /// hoist). Any non-minter caller is rejected before the body runs. function test_batchMint_revert_unauthorized(address caller, address to, uint256 amount) public { _assumeValidCaller(caller); _assumeValidActor(to); diff --git a/test/unit/B20Security/batch/batchMint_revertOrder.t.sol b/test/unit/B20Security/batch/batchMint_revertOrder.t.sol index f2941dd8..59200a0a 100644 --- a/test/unit/B20Security/batch/batchMint_revertOrder.t.sol +++ b/test/unit/B20Security/batch/batchMint_revertOrder.t.sol @@ -10,16 +10,19 @@ import {B20Constants} from "test/lib/mocks/MockB20.sol"; /// @title Differential check-order tests for `batchMint` (security variant). /// /// @notice **Canonical order (Solidity reference):** -/// 1. LENGTH_MISMATCH (`recipients.length != amounts.length`) → `LengthMismatch` -/// 2. EMPTY_BATCH (`recipients.length == 0`) → `EmptyBatch` -/// 3..N. Per-element `_mint` checks (see `mint_revertOrder.t.sol`): -/// ROLE → ZERO-RECEIVER → PAUSE → POLICY → CAP +/// 1. PAUSE (`whenNotPaused(MINT)` modifier) → `ContractPaused` +/// 2. ROLE (`onlyRole(MINT_ROLE)` modifier) → `AccessControlUnauthorizedAccount` +/// 3. LENGTH_MISMATCH (`recipients.length != amounts.length`) → `LengthMismatch` +/// 4. EMPTY_BATCH (`recipients.length == 0`) → `EmptyBatch` +/// 5. ZERO-RECEIVER (per-element inline guard) → `InvalidReceiver` +/// 6..N. Per-element `_mint` body (see `mint_revertOrder.t.sol`): +/// POLICY → CAP /// /// The pairs within `_mint`'s body are already pinned by -/// `mint_revertOrder.t.sol`. This file only pins the two batchMint-specific -/// preconditions (LENGTH_MISMATCH, EMPTY_BATCH) against the first per-element -/// check they encounter (ROLE), which transitively guarantees they fire -/// before every `_mint` body check. +/// `mint_revertOrder.t.sol`. This file pins the batch-level +/// preconditions (PAUSE, ROLE, LENGTH_MISMATCH, EMPTY_BATCH) +/// against each other and against the first per-element check +/// they encounter. contract B20SecurityBatchMintRevertOrderTest is B20SecurityTest { address[] internal _emptyAddrs; uint256[] internal _emptyUints; @@ -33,27 +36,87 @@ contract B20SecurityBatchMintRevertOrderTest is B20SecurityTest { _twoUints.push(1); } - /// @notice LENGTH_MISMATCH beats ROLE (and transitively everything inside `_mint`). - /// @dev Caller lacks MINT_ROLE; arrays have mismatched lengths. LENGTH check - /// fires in `batchMint` body before `_mint` is invoked, so the role - /// modifier on `_mint` never runs. - function test_batchMint_revertOrder_lengthMismatch_beats_mintBody(address caller) public { + // --- Pairs where PAUSE wins (PAUSE is canonical first) --- + + /// @notice PAUSE beats ROLE. + function test_batchMint_revertOrder_pause_beats_role(address caller) public { _assumeValidCaller(caller); vm.assume(caller != admin); vm.assume(caller != minter); + _pause(IB20.PausableFeature.MINT); vm.prank(caller); - vm.expectRevert(abi.encodeWithSelector(IB20Security.LengthMismatch.selector, uint256(2), uint256(1))); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.MINT)); + security().batchMint(_twoAddrs, _twoUints); + } + + /// @notice PAUSE beats LENGTH_MISMATCH. + function test_batchMint_revertOrder_pause_beats_lengthMismatch() public { + _grantRole(B20Constants.MINT_ROLE, minter); + _pause(IB20.PausableFeature.MINT); + + vm.prank(minter); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.MINT)); security().batchMint(_twoAddrs, _twoUints); } - /// @notice EMPTY_BATCH beats ROLE (and transitively everything inside `_mint`). - function test_batchMint_revertOrder_emptyBatch_beats_mintBody(address caller) public { + /// @notice PAUSE beats EMPTY_BATCH. + function test_batchMint_revertOrder_pause_beats_emptyBatch() public { + _grantRole(B20Constants.MINT_ROLE, minter); + _pause(IB20.PausableFeature.MINT); + + vm.prank(minter); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.MINT)); + security().batchMint(_emptyAddrs, _emptyUints); + } + + // --- Pairs where ROLE wins (PAUSE not violated) --- + + /// @notice ROLE beats LENGTH_MISMATCH. + /// @dev Role modifier fires before the body's length check. + function test_batchMint_revertOrder_role_beats_lengthMismatch(address caller) public { _assumeValidCaller(caller); vm.assume(caller != admin); vm.assume(caller != minter); vm.prank(caller); + vm.expectRevert( + abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, B20Constants.MINT_ROLE) + ); + security().batchMint(_twoAddrs, _twoUints); + } + + /// @notice ROLE beats EMPTY_BATCH. + function test_batchMint_revertOrder_role_beats_emptyBatch(address caller) public { + _assumeValidCaller(caller); + vm.assume(caller != admin); + vm.assume(caller != minter); + + vm.prank(caller); + vm.expectRevert( + abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, B20Constants.MINT_ROLE) + ); + security().batchMint(_emptyAddrs, _emptyUints); + } + + // --- Pairs where LENGTH_MISMATCH / EMPTY_BATCH win (PAUSE + ROLE satisfied) --- + + /// @notice LENGTH_MISMATCH beats per-element `_mint` body checks. + /// @dev With role granted and pause not set, the length check in batchMint's + /// body fires before the per-element loop runs. + function test_batchMint_revertOrder_lengthMismatch_beats_mintBody() public { + _grantRole(B20Constants.MINT_ROLE, minter); + + vm.prank(minter); + vm.expectRevert(abi.encodeWithSelector(IB20Security.LengthMismatch.selector, uint256(2), uint256(1))); + security().batchMint(_twoAddrs, _twoUints); + } + + /// @notice EMPTY_BATCH beats per-element `_mint` body checks. + function test_batchMint_revertOrder_emptyBatch_beats_mintBody() public { + _grantRole(B20Constants.MINT_ROLE, minter); + + vm.prank(minter); vm.expectRevert(IB20Security.EmptyBatch.selector); security().batchMint(_emptyAddrs, _emptyUints); }