diff --git a/test/lib/mocks/MockB20.sol b/test/lib/mocks/MockB20.sol index a479881..93f0240 100644 --- a/test/lib/mocks/MockB20.sol +++ b/test/lib/mocks/MockB20.sol @@ -186,20 +186,21 @@ abstract contract MockB20 is IB20 { returns (bool) { _requireNonZeroActors(from, to); - if (!_isPrivileged()) { - // Allowance is consumed unconditionally outside the factory - // bootstrap window. Matches OZ ERC20 and the Rust precompile, - // both of which carve no exception for `msg.sender == from`. - _consumeAllowance(from, msg.sender, amount); - if (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); - } + // Allowance is consumed unconditionally — including during the factory + // bootstrap window (`_isPrivileged()`). Matches the Rust precompile, + // which carves no `privileged` exception for allowance accounting + // (BOP-230 / L-04); only the executor-policy check below is bypassed + // for a privileged caller. An infinite allowance is still not + // decremented (handled inside `_consumeAllowance`). + _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); @@ -235,13 +236,15 @@ abstract contract MockB20 is IB20 { returns (bool) { _requireNonZeroActors(from, to); - if (!_isPrivileged()) { - _consumeAllowance(from, msg.sender, amount); - if (msg.sender != from) { - uint64 executorPolicyId = MockB20Storage.layout().transferPolicyIds.executor; - if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(executorPolicyId, msg.sender)) { - revert PolicyForbids(TRANSFER_EXECUTOR_POLICY, executorPolicyId); - } + // Allowance is consumed unconditionally — including during the factory + // bootstrap window — matching the Rust precompile (BOP-230 / L-04). + // Only the executor-policy check below is bypassed for a privileged + // caller; infinite allowance is still not decremented. + _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); @@ -702,10 +705,12 @@ abstract contract MockB20 is IB20 { /// 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. + /// additionally consume the allowance (unconditionally — + /// including in the bootstrap window, matching the Rust + /// precompile, see BOP-230 / L-04) 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. function _transfer(address from, address to, uint256 amount) internal { if (!_isPrivileged()) { // One SLOAD pulls both policy IDs we need for the transfer diff --git a/test/unit/B20/erc20/transferFrom.t.sol b/test/unit/B20/erc20/transferFrom.t.sol index 78a55ed..05a4174 100644 --- a/test/unit/B20/erc20/transferFrom.t.sol +++ b/test/unit/B20/erc20/transferFrom.t.sol @@ -364,4 +364,91 @@ contract B20TransferFromTest is B20Test { assertEq(token.balanceOf(to), amount, "transfer must succeed despite blocked executor policy"); } + + // ============================================================ + // REGRESSION: PRIVILEGED BOOTSTRAP ALLOWANCE (BOP-230 / L-04) + // ============================================================ + // + // A privileged transferFrom (factory caller during the bootstrap + // window) consumes allowance exactly like an ordinary transferFrom: + // the allowance is both CHECKED and DECREMENTED. The Rust precompile + // carves no `privileged` exception for allowance accounting — only the + // executor-policy check is bypassed for a privileged caller. Before + // BOP-230 the Solidity reference skipped the entire allowance block + // during the window (neither checking nor decrementing); these tests + // pin the corrected, Rust-aligned behavior. + // + // To enter the window: set allowance/balance while still initialized, + // then reopen the bootstrap window via vm.store on the initialized + // slot, then call as the factory. The privileged spender is the + // factory address. + + /// @notice Verifies a privileged transferFrom reverts InsufficientAllowance when allowance is below the spend + /// @dev Pins that the allowance check is unconditional — a privileged caller is still + /// rejected for insufficient allowance; before BOP-230 the privileged path skipped + /// the check entirely. Regression: BOP-230 / L-04. + function test_transferFrom_revert_privileged_insufficientAllowance( + address from, + address to, + uint256 allowanceAmount, + uint256 spendAmount + ) public { + _assumeValidActor(from); + _assumeValidActor(to); + allowanceAmount = bound(allowanceAmount, 0, type(uint128).max - 1); + spendAmount = bound(spendAmount, allowanceAmount + 1, type(uint128).max); + + _mint(from, spendAmount); + vm.prank(from); + token.approve(address(factory), allowanceAmount); + + // Reopen the factory bootstrap window so the factory caller is privileged. + vm.store(address(token), MockB20Storage.initializedSlot(), bytes32(0)); + + vm.prank(address(factory)); + vm.expectRevert( + abi.encodeWithSelector(IB20.InsufficientAllowance.selector, address(factory), allowanceAmount, spendAmount) + ); + token.transferFrom(from, to, spendAmount); + } + + /// @notice Verifies a privileged transferFrom decrements allowance by the spent amount + /// @dev Allowance is consumed during the bootstrap window exactly as outside it, matching + /// the Rust precompile. Before BOP-230 the privileged path left the allowance + /// untouched. Regression: BOP-230 / L-04. + function test_transferFrom_success_privileged_decrementsAllowance( + address from, + address to, + uint256 allowanceAmount, + uint256 spendAmount + ) public { + _assumeValidActor(from); + _assumeValidActor(to); + vm.assume(from != to); + allowanceAmount = bound(allowanceAmount, 1, type(uint128).max); + vm.assume(allowanceAmount != type(uint256).max); + spendAmount = bound(spendAmount, 0, allowanceAmount); + + _mint(from, spendAmount); + vm.prank(from); + token.approve(address(factory), allowanceAmount); + + // Reopen the factory bootstrap window so the factory caller is privileged. + vm.store(address(token), MockB20Storage.initializedSlot(), bytes32(0)); + + vm.prank(address(factory)); + token.transferFrom(from, to, spendAmount); + + assertEq( + token.allowance(from, address(factory)), + allowanceAmount - spendAmount, + "privileged transferFrom must decrement allowance by the spent amount" + ); + assertEq(token.balanceOf(to), spendAmount, "to must receive the spent amount"); + assertEq( + uint256(vm.load(address(token), MockB20Storage.allowanceSlot(from, address(factory)))), + allowanceAmount - spendAmount, + "allowances[from][factory] slot must reflect the consumed amount" + ); + } } diff --git a/test/unit/B20/memo/transferFromWithMemo.t.sol b/test/unit/B20/memo/transferFromWithMemo.t.sol index 759c2ab..65f2f0e 100644 --- a/test/unit/B20/memo/transferFromWithMemo.t.sol +++ b/test/unit/B20/memo/transferFromWithMemo.t.sol @@ -177,4 +177,85 @@ contract B20TransferFromWithMemoTest is B20Test { ); assertEq(token.balanceOf(to), amount, "to must receive the transferred amount"); } + + // ============================================================ + // REGRESSION: PRIVILEGED BOOTSTRAP ALLOWANCE (BOP-230 / L-04) + // ============================================================ + // + // Mirrors the transferFrom regression: a privileged transferFromWithMemo + // (factory caller during the bootstrap window) consumes allowance exactly + // like an ordinary call — both checked and decremented — while only the + // executor-policy check stays bypassed, matching the Rust precompile. See + // transferFrom.t.sol for the canonical discussion. + + /// @notice Verifies a privileged transferFromWithMemo reverts InsufficientAllowance when allowance is below the spend + /// @dev Pins that the allowance check is unconditional — a privileged caller is still + /// rejected for insufficient allowance; before BOP-230 the privileged path skipped + /// the check entirely. Regression: BOP-230 / L-04. + function test_transferFromWithMemo_revert_privileged_insufficientAllowance( + address from, + address to, + uint256 allowanceAmount, + uint256 spendAmount, + bytes32 memo + ) public { + _assumeValidActor(from); + _assumeValidActor(to); + allowanceAmount = bound(allowanceAmount, 0, type(uint128).max - 1); + spendAmount = bound(spendAmount, allowanceAmount + 1, type(uint128).max); + + _mint(from, spendAmount); + vm.prank(from); + token.approve(address(factory), allowanceAmount); + + // Reopen the factory bootstrap window so the factory caller is privileged. + vm.store(address(token), MockB20Storage.initializedSlot(), bytes32(0)); + + vm.prank(address(factory)); + vm.expectRevert( + abi.encodeWithSelector(IB20.InsufficientAllowance.selector, address(factory), allowanceAmount, spendAmount) + ); + token.transferFromWithMemo(from, to, spendAmount, memo); + } + + /// @notice Verifies a privileged transferFromWithMemo decrements allowance by the spent amount + /// @dev Allowance is consumed during the bootstrap window exactly as outside it, matching + /// the Rust precompile. Before BOP-230 the privileged path left the allowance + /// untouched. Regression: BOP-230 / L-04. + function test_transferFromWithMemo_success_privileged_decrementsAllowance( + address from, + address to, + uint256 allowanceAmount, + uint256 spendAmount, + bytes32 memo + ) public { + _assumeValidActor(from); + _assumeValidActor(to); + vm.assume(from != to); + allowanceAmount = bound(allowanceAmount, 1, type(uint128).max); + vm.assume(allowanceAmount != type(uint256).max); + spendAmount = bound(spendAmount, 0, allowanceAmount); + + _mint(from, spendAmount); + vm.prank(from); + token.approve(address(factory), allowanceAmount); + + // Reopen the factory bootstrap window so the factory caller is privileged. + vm.store(address(token), MockB20Storage.initializedSlot(), bytes32(0)); + + vm.prank(address(factory)); + token.transferFromWithMemo(from, to, spendAmount, memo); + + assertEq( + token.allowance(from, address(factory)), + allowanceAmount - spendAmount, + "privileged transferFromWithMemo must decrement allowance by the spent amount" + ); + assertEq(token.balanceOf(to), spendAmount, "to must receive the spent amount"); + assertEq( + uint256(vm.load(address(token), MockB20Storage.allowanceSlot(from, address(factory)))), + allowanceAmount - spendAmount, + "allowances[from][factory] slot must reflect the consumed amount" + ); + } }