diff --git a/src/interfaces/IB20.sol b/src/interfaces/IB20.sol index 60d9661..ac90828 100644 --- a/src/interfaces/IB20.sol +++ b/src/interfaces/IB20.sol @@ -248,8 +248,8 @@ interface IB20 { /// @return Policy scope constant. function TRANSFER_RECEIVER_POLICY() external view returns (bytes32); - /// @notice Policy slot consulted against `msg.sender` on `transferFrom` when distinct from `from`. - /// Not consulted on `transfer`. + /// @notice Policy slot consulted against `msg.sender` (the initiator) on every transfer, + /// including when `msg.sender == from`. /// @dev Bypassed for factory-originated calls during the creation (bootstrap) window; see /// `IB20Factory.createB20`. /// @return Policy scope constant. @@ -317,6 +317,7 @@ interface IB20 { /// @dev Reverts with `ContractPaused(TRANSFER)` when `TRANSFER` is paused. /// @dev Reverts with `InvalidReceiver` when `to == address(0)`. /// @dev Reverts with `InvalidSender` when `msg.sender == address(0)`. + /// @dev Reverts with `PolicyForbids(TRANSFER_EXECUTOR_POLICY, ...)` when `msg.sender` is not authorized. /// @dev Reverts with `PolicyForbids(TRANSFER_SENDER_POLICY, ...)` when `msg.sender` is not authorized. /// @dev Reverts with `PolicyForbids(TRANSFER_RECEIVER_POLICY, ...)` when `to` is not authorized. /// @dev Reverts with `InsufficientBalance` when `msg.sender`'s balance is below `amount`. @@ -333,7 +334,7 @@ interface IB20 { /// @dev Reverts with `InvalidReceiver` when `to == address(0)`. /// @dev Reverts with `InvalidSender` when `from == address(0)`. /// @dev Reverts with `InsufficientAllowance` when the caller's allowance from `from` is below `amount`. - /// @dev Reverts with `PolicyForbids(TRANSFER_EXECUTOR_POLICY, ...)` when `msg.sender != from` and `msg.sender` is not authorized. + /// @dev Reverts with `PolicyForbids(TRANSFER_EXECUTOR_POLICY, ...)` when `msg.sender` is not authorized. /// @dev Reverts with `PolicyForbids(TRANSFER_SENDER_POLICY, ...)` when `from` is not authorized. /// @dev Reverts with `PolicyForbids(TRANSFER_RECEIVER_POLICY, ...)` when `to` is not authorized. /// @dev Reverts with `InsufficientBalance` when `from`'s balance is below `amount`. diff --git a/test/lib/mocks/MockB20.sol b/test/lib/mocks/MockB20.sol index 05142a2..45f9925 100644 --- a/test/lib/mocks/MockB20.sol +++ b/test/lib/mocks/MockB20.sol @@ -198,21 +198,11 @@ abstract contract MockB20 is IB20 { _requireNonZeroActors(from, to); // Allowance is consumed unconditionally — including during the factory // bootstrap window (`_isPrivileged()`). Matches the Rust precompile, - // which carves no `privileged` exception for allowance accounting; - // only the executor-policy check below is bypassed - // for a privileged caller. An infinite allowance is still not - // decremented (handled inside `_consumeAllowance`). + // which carves no `privileged` exception for allowance accounting. An + // infinite allowance is still not decremented (handled inside + // `_consumeAllowance`). The executor policy is enforced centrally in + // `_transfer` (on `msg.sender`), which honors the bootstrap bypass. _consumeAllowance(from, msg.sender, amount); - if (!_isPrivileged() && msg.sender != from) { - // Read the executor policy ID out of the transfer-side packed - // slot. Cold here; warm by the time _transfer reads the same - // slot for sender + receiver. Skipped when the caller is the - // owner — sender-policy already covers `from` inside _transfer. - uint64 executorPolicyId = MockB20Storage.layout().transferPolicyIds.executor; - if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(executorPolicyId, msg.sender)) { - revert PolicyForbids(TRANSFER_EXECUTOR_POLICY, executorPolicyId); - } - } _transfer(from, to, amount); return true; } @@ -247,16 +237,10 @@ abstract contract MockB20 is IB20 { { _requireNonZeroActors(from, to); // Allowance is consumed unconditionally — including during the factory - // bootstrap window — matching the Rust precompile. - // Only the executor-policy check below is bypassed for a privileged - // caller; infinite allowance is still not decremented. + // bootstrap window — matching the Rust precompile. Infinite allowance + // is still not decremented. The executor policy is enforced centrally + // in `_transfer` (on `msg.sender`), which honors the bootstrap bypass. _consumeAllowance(from, msg.sender, amount); - if (!_isPrivileged() && msg.sender != from) { - uint64 executorPolicyId = MockB20Storage.layout().transferPolicyIds.executor; - if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(executorPolicyId, msg.sender)) { - revert PolicyForbids(TRANSFER_EXECUTOR_POLICY, executorPolicyId); - } - } _transfer(from, to, amount); emit Memo(msg.sender, memo); return true; @@ -754,18 +738,22 @@ abstract contract MockB20 is IB20 { /// this helper. `transferFrom` / `transferFromWithMemo` /// additionally consume the allowance (unconditionally — /// including in the bootstrap window, matching the Rust - /// precompile) and check the executor - /// policy in their bodies before calling here; only the - /// executor-policy check honors the bootstrap bypass, - /// consistent with the sender/receiver policy bypass below. + /// precompile) before calling here. + /// + /// Enforces the executor (`msg.sender`), sender (`from`), and receiver + /// (`to`) policies. Gating the executor here — not just on delegated + /// `transferFrom` — lets an executor allowlist restrict who may + /// initiate any transfer, including a holder moving their own tokens. + /// All honor the bootstrap bypass; an unset lane is always-allow. function _transfer(address from, address to, uint256 amount) internal { if (!_isPrivileged()) { - // 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). - // Solidity emits a single SLOAD for the struct read + masked - // extracts for the named fields. + // One SLOAD pulls all three policy IDs we need for the transfer + // check. Solidity emits a single SLOAD for the struct read + + // masked extracts for the named fields. MockB20Storage.TransferPolicyIds memory packed = MockB20Storage.layout().transferPolicyIds; + if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(packed.executor, msg.sender)) { + revert PolicyForbids(TRANSFER_EXECUTOR_POLICY, packed.executor); + } if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(packed.sender, from)) { revert PolicyForbids(TRANSFER_SENDER_POLICY, packed.sender); } diff --git a/test/lib/mocks/MockB20Storage.sol b/test/lib/mocks/MockB20Storage.sol index 393ecad..6c8f9ee 100644 --- a/test/lib/mocks/MockB20Storage.sol +++ b/test/lib/mocks/MockB20Storage.sol @@ -49,7 +49,7 @@ library MockB20Storage { // not declared as a field is simply uninitialized (zero) and the // struct cannot accidentally write to it. - /// @notice Transfer-side policy IDs (read by `_transfer` and `transferFrom*`). + /// @notice Transfer-side policy IDs (all three lanes read by `_transfer`). /// @dev Bit layout (Solidity LSB-first): /// bits 0.. 63 : sender /// bits 64..127 : receiver @@ -119,8 +119,8 @@ library MockB20Storage { // access (`$.transferPolicyIds.sender = id;`) instead of inline // shifts and mask operations. // - // Transfer-side policies (read by `_transfer`, `transferFrom*`, - // and the blocked check in the deprecated `burnBlocked`). + // Transfer-side policies (read by `_transfer`, and the sender lane + // by the blocked check in the deprecated `burnBlocked`). TransferPolicyIds transferPolicyIds; // Mint-side policies (read by `_mint`). Only `MINT_RECEIVER_POLICY` // is defined today; future granular mint-side policy types (e.g. diff --git a/test/unit/B20/erc20/transfer.t.sol b/test/unit/B20/erc20/transfer.t.sol index cb9f91d..3cb5299 100644 --- a/test/unit/B20/erc20/transfer.t.sol +++ b/test/unit/B20/erc20/transfer.t.sol @@ -60,6 +60,28 @@ contract B20TransferTest is B20Test { token.transfer(to, amount); } + /// @notice Verifies transfer reverts when the executor (msg.sender) is not authorized under + /// TRANSFER_EXECUTOR_POLICY + /// @dev On the direct `transfer` path the executor is `msg.sender` (== `from`). The executor + /// gate is enforced in `_transfer` before the sender/receiver gates, so a blocked executor + /// reverts PolicyForbids(TRANSFER_EXECUTOR_POLICY, ...) even for a holder moving their own + /// tokens. No balance needed — the policy check fires first. + function test_transfer_revert_executorPolicyForbids(address from, address to, uint256 amount) public { + _assumeValidActor(from); + _assumeValidActor(to); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(from); + vm.expectRevert( + abi.encodeWithSelector( + IB20.PolicyForbids.selector, + B20Constants.TRANSFER_EXECUTOR_POLICY, + PolicyRegistryConstants.ALWAYS_BLOCK_ID + ) + ); + token.transfer(to, amount); + } + /// @notice Verifies transfer reverts when sender balance is insufficient /// @dev Balance precondition; checks InsufficientBalance(sender, balance, amount) error function test_transfer_revert_insufficientBalance(address from, address to, uint256 amount) public { @@ -299,6 +321,74 @@ contract B20TransferTest is B20Test { token.transfer(to, amount); } + /// @notice Verifies transfer succeeds when the executor (msg.sender) is a member of a custom + /// ALLOWLIST policy + /// @dev Exercises the external-registry authorization path for the executor scope: only an + /// allowlisted initiator can move tokens. Here the holder `from` is on the allowlist, so + /// their own `transfer` clears the executor gate. + function test_transfer_success_externalExecutorPolicyAllows(address from, address to, uint256 amount) public { + _assumeValidActor(from); + _assumeValidActor(to); + vm.assume(from != to); + amount = bound(amount, 0, B20Constants.MAX_SUPPLY_CAP); + + uint64 id = _createAllowlist(from, true); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, id); + _mint(from, amount); + + vm.prank(from); + token.transfer(to, amount); + + assertEq(token.balanceOf(to), amount, "transfer must succeed when executor is allowlisted"); + } + + /// @notice Verifies transfer reverts when the executor (msg.sender) is NOT a member of a custom + /// ALLOWLIST policy + /// @dev Negative external-registry path for the executor scope: an allowlist without membership + /// for `from` resolves isAuthorized to false, so the executor gate reverts PolicyForbids + /// with the custom id. This is the case an issuer uses to restrict transfers to specific + /// initiators (e.g. a settlement contract). No balance needed — the policy check fires first. + function test_transfer_revert_externalExecutorPolicyDenies(address from, address to, uint256 amount) public { + _assumeValidActor(from); + _assumeValidActor(to); + vm.assume(from != to); + + uint64 id = _createAllowlist(from, false); // create the allowlist but do NOT add `from` + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, id); + + vm.prank(from); + vm.expectRevert(abi.encodeWithSelector(IB20.PolicyForbids.selector, B20Constants.TRANSFER_EXECUTOR_POLICY, id)); + token.transfer(to, amount); + } + + /// @notice Verifies a privileged (factory bootstrap) transfer bypasses the TRANSFER_EXECUTOR_POLICY + /// @dev Executor mirror of the sender/receiver bootstrap bypasses: the initCalls set the executor + /// policy to ALWAYS_BLOCK and transfer from the factory. A non-privileged transfer would + /// revert PolicyForbids(EXECUTOR, ...); the privileged init-call transfer must succeed, + /// proving the executor gate honors the bootstrap bypass on the direct transfer path. Runs + /// the real factory bootstrap path with no vm.store cheat, so it holds under LIVE_PRECOMPILES. + function test_transfer_success_privilegedBypassesExecutorPolicy(address to, uint256 amount) public { + _assumeValidActor(to); + amount = bound(amount, 0, B20Constants.MAX_SUPPLY_CAP); + + bytes32 salt = keccak256("privileged-executor-bypass"); + // The fuzzed recipient must not collide with the to-be-created token's own address. + vm.assume(to != factory.getB20Address(IB20Factory.B20Variant.ASSET, alice, salt)); + + bytes[] memory initCalls = new bytes[](3); + initCalls[0] = abi.encodeWithSelector(IB20.mint.selector, address(factory), amount); + initCalls[1] = abi.encodeWithSelector( + IB20.updatePolicy.selector, B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID + ); + initCalls[2] = abi.encodeWithSelector(IB20.transfer.selector, to, amount); + + address newToken = _createAsset(alice, salt, _assetParams(), initCalls); + + assertEq( + IB20(newToken).balanceOf(to), amount, "privileged transfer must succeed despite blocked executor policy" + ); + } + /// @notice Creates a custom ALLOWLIST policy administered by `admin`, optionally seeding /// `member`, and returns its id. Drives the external-registry authorization path /// (custom policy id) beyond the ALWAYS_ALLOW / ALWAYS_BLOCK sentinels. diff --git a/test/unit/B20/erc20/transferFrom.t.sol b/test/unit/B20/erc20/transferFrom.t.sol index 0b3197a..9c96bee 100644 --- a/test/unit/B20/erc20/transferFrom.t.sol +++ b/test/unit/B20/erc20/transferFrom.t.sol @@ -347,11 +347,15 @@ contract B20TransferFromTest is B20Test { assertEq(token.balanceOf(to), spendAmount, "to must receive the spent amount"); } - /// @notice Verifies transferFrom with self-caller skips the executor policy check - /// @dev Self-caller is not an executor distinct from `from`; sender-policy already - /// covers `from` inside _transfer. Executor policy MUST NOT fire — pins the - /// one carve-out we intentionally keep around `msg.sender == from`. - function test_transferFrom_success_selfCaller_skipsExecutorPolicy(address from, address to, uint256 amount) public { + /// @notice Verifies transferFrom with a self-caller is still gated by the executor policy + /// @dev Executor enforcement is centralized in `_transfer` on `msg.sender`, so the old + /// `msg.sender == from` carve-out is gone: a holder moving their own tokens via + /// transferFrom must also clear TRANSFER_EXECUTOR_POLICY. This closes the bypass where + /// an executor allowlist could be sidestepped by routing a self-transferFrom. Allowance + /// is self-approved so the executor check — not the allowance gate — is what fires. + function test_transferFrom_revert_selfCaller_executorPolicyForbids(address from, address to, uint256 amount) + public + { _assumeValidActor(from); _assumeValidActor(to); vm.assume(from != to); @@ -363,9 +367,14 @@ contract B20TransferFromTest is B20Test { _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); vm.prank(from); + vm.expectRevert( + abi.encodeWithSelector( + IB20.PolicyForbids.selector, + B20Constants.TRANSFER_EXECUTOR_POLICY, + PolicyRegistryConstants.ALWAYS_BLOCK_ID + ) + ); token.transferFrom(from, to, amount); - - assertEq(token.balanceOf(to), amount, "transfer must succeed despite blocked executor policy"); } // ============================================================ diff --git a/test/unit/B20/erc20/transferFrom_revertOrder.t.sol b/test/unit/B20/erc20/transferFrom_revertOrder.t.sol index ba84c27..3f2ec85 100644 --- a/test/unit/B20/erc20/transferFrom_revertOrder.t.sol +++ b/test/unit/B20/erc20/transferFrom_revertOrder.t.sol @@ -9,28 +9,28 @@ import {PolicyRegistryConstants} from "base-std-test/lib/mocks/MockPolicyRegistr /// @title Differential check-order tests for `transferFrom`. /// -/// @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. +/// @notice `transferFrom` consumes the allowance in the entrypoint body, then +/// defers to `_transfer` for the policy / balance checks. The PAUSE / +/// ZERO-RECEIVER / ZERO-SENDER guards run before the allowance work in +/// the entrypoint body; the EXECUTOR / SENDER / RECEIVER / BALANCE +/// checks all run inside `_transfer`, with EXECUTOR first. /// -/// **Canonical order (Solidity reference, when -/// `msg.sender != from`):** +/// **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. ALLOWANCE (`_consumeAllowance`) → `InsufficientAllowance` -/// 5. EXECUTOR-POLICY (`isAuthorized(executorPolicyId, msg.sender)`) +/// 5. EXECUTOR-POLICY (`_transfer` body: `isAuthorized(executor, msg.sender)`) /// → `PolicyForbids(EXECUTOR, ...)` -/// 6..N. All `_transfer` body checks — see `transfer_revertOrder.t.sol` +/// 6..N. Remaining `_transfer` body checks — see `transfer_revertOrder.t.sol` /// (SENDER-POLICY → RECEIVER-POLICY → BALANCE). /// -/// 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. +/// The executor gate is enforced on every transfer path, not only when +/// `msg.sender != from`; this suite exercises the delegated path with a +/// distinct caller (the self-caller case is pinned in `transferFrom.t.sol`). +/// The pair matrix between the body-level ALLOWANCE guard, the +/// PAUSE/ZERO-RECEIVER/ZERO-SENDER guards, and the leading `_transfer` +/// EXECUTOR-POLICY check is pinned below. contract B20TransferFromRevertOrderTest is B20Test { // --- Pairs where PAUSE wins (PAUSE is canonical first) --- @@ -186,10 +186,9 @@ contract B20TransferFromRevertOrderTest is B20Test { // --- 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. + /// @notice EXECUTOR-POLICY beats the other `_transfer` checks (representative: SENDER-POLICY). + /// @dev Allowance is set high enough to pass the allowance check, so `_transfer` is entered; + /// the executor gate is checked first inside `_transfer` and fires before SENDER-POLICY. function test_transferFrom_revertOrder_executorPolicy_beats_transferBody( address caller, address from, diff --git a/test/unit/B20/erc20/transferWithMemo_revertOrder.t.sol b/test/unit/B20/erc20/transferWithMemo_revertOrder.t.sol index 2b331e1..50dadbf 100644 --- a/test/unit/B20/erc20/transferWithMemo_revertOrder.t.sol +++ b/test/unit/B20/erc20/transferWithMemo_revertOrder.t.sol @@ -16,24 +16,27 @@ import {PolicyRegistryConstants} from "base-std-test/lib/mocks/MockPolicyRegistr /// 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` +/// 4. EXECUTOR-POLICY (`_transfer` body) → `PolicyForbids(EXECUTOR, ...)` +/// 5. SENDER-POLICY (`_transfer` body) → `PolicyForbids(SENDER, ...)` +/// 6. RECEIVER-POLICY (`_transfer` body) → `PolicyForbids(RECEIVER, ...)` +/// 7. BALANCE (`_transfer` body) → `InsufficientBalance` /// /// The public `transferWithMemo(to, amount, memo)` entry sets -/// `from = msg.sender`. The single test below activates all six -/// violations simultaneously, then fixes them one at a time in -/// canonical order, asserting that the next-priority revert fires -/// at each step. +/// `from = msg.sender`, so the executor is `msg.sender` (== `from`). +/// The single test below activates all seven violations +/// simultaneously, then fixes them one at a time in canonical order, +/// asserting that the next-priority revert fires at each step. contract B20TransferWithMemoRevertOrderTest is B20Test { function test_transferWithMemo_revertOrder(address from, address to, uint256 amount, bytes32 memo) public { _assumeValidActor(from); _assumeValidActor(to); amount = bound(amount, 1, type(uint128).max); - // Activate all six violations: TRANSFER paused, from=address(0) (via prank), - // to=address(0), sender policy blocks, receiver policy blocks, from has zero balance. + // Activate all seven violations: TRANSFER paused, from=address(0) (via prank), + // to=address(0), executor policy blocks, sender policy blocks, receiver policy blocks, + // from has zero balance. _pause(IB20.PausableFeature.TRANSFER); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); _setPolicy(B20Constants.TRANSFER_SENDER_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); _setPolicy(B20Constants.TRANSFER_RECEIVER_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); @@ -56,7 +59,20 @@ contract B20TransferWithMemoRevertOrderTest is B20Test { vm.expectRevert(abi.encodeWithSelector(IB20.InvalidSender.selector, address(0))); token.transferWithMemo(to, amount, memo); - // 4. SENDER-POLICY fires (all earlier cleared; sender policy blocks, receiver also blocks). + // 4. EXECUTOR-POLICY fires (all earlier cleared; executor == msg.sender == from is + // blocked; sender/receiver also block, but executor is checked first in _transfer). + vm.prank(from); + vm.expectRevert( + abi.encodeWithSelector( + IB20.PolicyForbids.selector, + B20Constants.TRANSFER_EXECUTOR_POLICY, + PolicyRegistryConstants.ALWAYS_BLOCK_ID + ) + ); + token.transferWithMemo(to, amount, memo); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_ALLOW_ID); + + // 5. SENDER-POLICY fires (all earlier cleared; sender policy blocks, receiver also blocks). vm.prank(from); vm.expectRevert( abi.encodeWithSelector( @@ -68,7 +84,7 @@ contract B20TransferWithMemoRevertOrderTest is B20Test { token.transferWithMemo(to, amount, memo); _setPolicy(B20Constants.TRANSFER_SENDER_POLICY, PolicyRegistryConstants.ALWAYS_ALLOW_ID); - // 5. RECEIVER-POLICY fires (all earlier cleared; receiver policy still blocks). + // 6. RECEIVER-POLICY fires (all earlier cleared; receiver policy still blocks). vm.prank(from); vm.expectRevert( abi.encodeWithSelector( @@ -80,7 +96,7 @@ contract B20TransferWithMemoRevertOrderTest is B20Test { token.transferWithMemo(to, amount, memo); _setPolicy(B20Constants.TRANSFER_RECEIVER_POLICY, PolicyRegistryConstants.ALWAYS_ALLOW_ID); - // 6. BALANCE fires (all earlier cleared; from has zero balance, amount>0). + // 7. BALANCE fires (all earlier cleared; from has zero balance, amount>0). vm.prank(from); vm.expectRevert(abi.encodeWithSelector(IB20.InsufficientBalance.selector, from, 0, amount)); token.transferWithMemo(to, amount, memo); diff --git a/test/unit/B20/erc20/transfer_revertOrder.t.sol b/test/unit/B20/erc20/transfer_revertOrder.t.sol index 839e8ee..1d39858 100644 --- a/test/unit/B20/erc20/transfer_revertOrder.t.sol +++ b/test/unit/B20/erc20/transfer_revertOrder.t.sol @@ -13,12 +13,14 @@ import {PolicyRegistryConstants} from "base-std-test/lib/mocks/MockPolicyRegistr /// 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` +/// 4. EXECUTOR-POLICY (`_transfer` body) → `PolicyForbids(EXECUTOR, ...)` +/// 5. SENDER-POLICY (`_transfer` body) → `PolicyForbids(SENDER, ...)` +/// 6. RECEIVER-POLICY (`_transfer` body) → `PolicyForbids(RECEIVER, ...)` +/// 7. 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. +/// The public `transfer(to, amount)` entry sets `from = msg.sender`, so the executor +/// is `msg.sender` (== `from`): a blocked EXECUTOR policy reverts even on this direct +/// path. Pairs involving ZERO-SENDER require pranking `address(0)`. C(7, 2) = 21 pairs. contract B20TransferRevertOrderTest is B20Test { // --- Pairs where PAUSE wins (PAUSE is canonical first) --- @@ -49,6 +51,15 @@ contract B20TransferRevertOrderTest is B20Test { token.transfer(address(0), amount); } + function test_transfer_revertOrder_zeroReceiver_beats_executorPolicy(address from, uint256 amount) public { + _assumeValidActor(from); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(from); + vm.expectRevert(abi.encodeWithSelector(IB20.InvalidReceiver.selector, address(0))); + token.transfer(address(0), amount); + } + function test_transfer_revertOrder_zeroReceiver_beats_senderPolicy(address from, uint256 amount) public { _assumeValidActor(from); _setPolicy(B20Constants.TRANSFER_SENDER_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); @@ -79,6 +90,15 @@ contract B20TransferRevertOrderTest is B20Test { // --- Pairs where ZERO-SENDER wins (PAUSE not violated; requires pranking address(0)) --- + function test_transfer_revertOrder_zeroSender_beats_executorPolicy(address to, uint256 amount) public { + _assumeValidActor(to); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(address(0)); + vm.expectRevert(abi.encodeWithSelector(IB20.InvalidSender.selector, address(0))); + token.transfer(to, amount); + } + function test_transfer_revertOrder_zeroSender_beats_senderPolicy(address to, uint256 amount) public { _assumeValidActor(to); _setPolicy(B20Constants.TRANSFER_SENDER_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); @@ -119,6 +139,17 @@ contract B20TransferRevertOrderTest is B20Test { token.transfer(to, amount); } + function test_transfer_revertOrder_pause_beats_executorPolicy(address from, address to, uint256 amount) public { + _assumeValidActor(from); + _assumeValidActor(to); + _pause(IB20.PausableFeature.TRANSFER); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(from); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.TRANSFER)); + token.transfer(to, amount); + } + function test_transfer_revertOrder_pause_beats_receiverPolicy(address from, address to, uint256 amount) public { _assumeValidActor(from); _assumeValidActor(to); @@ -141,6 +172,63 @@ contract B20TransferRevertOrderTest is B20Test { token.transfer(to, amount); } + // --- Pairs where EXECUTOR-POLICY wins (executor == msg.sender == from) --- + + function test_transfer_revertOrder_executorPolicy_beats_senderPolicy(address from, address to, uint256 amount) + public + { + _assumeValidActor(from); + _assumeValidActor(to); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + _setPolicy(B20Constants.TRANSFER_SENDER_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(from); + vm.expectRevert( + abi.encodeWithSelector( + IB20.PolicyForbids.selector, + B20Constants.TRANSFER_EXECUTOR_POLICY, + PolicyRegistryConstants.ALWAYS_BLOCK_ID + ) + ); + token.transfer(to, amount); + } + + function test_transfer_revertOrder_executorPolicy_beats_receiverPolicy(address from, address to, uint256 amount) + public + { + _assumeValidActor(from); + _assumeValidActor(to); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + _setPolicy(B20Constants.TRANSFER_RECEIVER_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(from); + vm.expectRevert( + abi.encodeWithSelector( + IB20.PolicyForbids.selector, + B20Constants.TRANSFER_EXECUTOR_POLICY, + PolicyRegistryConstants.ALWAYS_BLOCK_ID + ) + ); + token.transfer(to, amount); + } + + function test_transfer_revertOrder_executorPolicy_beats_balance(address from, address to, uint256 amount) public { + _assumeValidActor(from); + _assumeValidActor(to); + amount = bound(amount, 1, type(uint128).max); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(from); + vm.expectRevert( + abi.encodeWithSelector( + IB20.PolicyForbids.selector, + B20Constants.TRANSFER_EXECUTOR_POLICY, + PolicyRegistryConstants.ALWAYS_BLOCK_ID + ) + ); + token.transfer(to, amount); + } + // --- Pairs where SENDER-POLICY wins --- function test_transfer_revertOrder_senderPolicy_beats_receiverPolicy(address from, address to, uint256 amount) diff --git a/test/unit/B20/memo/transferWithMemo.t.sol b/test/unit/B20/memo/transferWithMemo.t.sol index 3af4bb7..3c95b92 100644 --- a/test/unit/B20/memo/transferWithMemo.t.sol +++ b/test/unit/B20/memo/transferWithMemo.t.sol @@ -6,6 +6,7 @@ import {IB20} from "base-std/interfaces/IB20.sol"; import {B20Test} from "base-std-test/lib/B20Test.sol"; import {B20Constants} from "base-std-test/lib/mocks/MockB20.sol"; import {MockB20Storage} from "base-std-test/lib/mocks/MockB20Storage.sol"; +import {PolicyRegistryConstants} from "base-std-test/lib/mocks/MockPolicyRegistry.sol"; contract B20TransferWithMemoTest is B20Test { /// @notice Verifies transferWithMemo applies the same pause / policy / balance checks as transfer @@ -24,6 +25,28 @@ contract B20TransferWithMemoTest is B20Test { token.transferWithMemo(to, amount, memo); } + /// @notice Verifies transferWithMemo enforces TRANSFER_EXECUTOR_POLICY like transfer + /// @dev The memo variant routes through the same `_transfer`, so a blocked executor + /// (msg.sender == from) must revert PolicyForbids(TRANSFER_EXECUTOR_POLICY, ...). + /// Concrete executor-scope tests live in transfer.t.sol; this pins parity for the memo path. + function test_transferWithMemo_revert_executorPolicyForbids(address from, address to, uint256 amount, bytes32 memo) + public + { + _assumeValidActor(from); + _assumeValidActor(to); + _setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(from); + vm.expectRevert( + abi.encodeWithSelector( + IB20.PolicyForbids.selector, + B20Constants.TRANSFER_EXECUTOR_POLICY, + PolicyRegistryConstants.ALWAYS_BLOCK_ID + ) + ); + token.transferWithMemo(to, amount, memo); + } + /// @notice Verifies transferWithMemo performs the same balance movement as transfer /// @dev Same accounting effect as transfer; the memo does not alter accounting. /// Paired slot assertions confirm both balance slots reflect the move.