Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions test/lib/mocks/MockB20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,20 @@ contract MockB20 is IB20 {
}

function transferFrom(address from, address to, uint256 amount) external returns (bool) {
if (!_isPrivileged() && msg.sender != from) {
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);
// 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.
uint64 executorPolicyId = MockB20Storage.layout().transferPolicyIds.executor;
if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(executorPolicyId, msg.sender)) {
revert PolicyForbids(TRANSFER_EXECUTOR_POLICY, executorPolicyId);
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);
}
}
}
_transfer(from, to, amount);
Expand All @@ -187,11 +193,13 @@ contract MockB20 is IB20 {
}

function transferFromWithMemo(address from, address to, uint256 amount, bytes32 memo) external returns (bool) {
if (!_isPrivileged() && msg.sender != from) {
if (!_isPrivileged()) {
_consumeAllowance(from, msg.sender, amount);
uint64 executorPolicyId = MockB20Storage.layout().transferPolicyIds.executor;
if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(executorPolicyId, msg.sender)) {
revert PolicyForbids(TRANSFER_EXECUTOR_POLICY, executorPolicyId);
if (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);
Expand Down
80 changes: 80 additions & 0 deletions test/unit/B20/erc20/transferFrom.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -280,4 +280,84 @@ contract B20TransferFromTest is B20Test {
vm.prank(caller);
assertTrue(token.transferFrom(from, to, amount), "transferFrom must return true");
}

// ============================================================
// REGRESSION: SELF-CALLER ALLOWANCE
// ============================================================
//
// transferFrom MUST consume allowance even when msg.sender == from.
// OZ ERC20 and the Rust precompile both carve no exception for the
// self-caller case, so the mock must not either. Earlier the mock
// gated `_consumeAllowance` behind `msg.sender != from`, silently
// letting an owner-as-caller transfer without burning approval —
// these tests pin the corrected behavior.

/// @notice Verifies transferFrom reverts InsufficientAllowance when caller == from and no self-approval exists
/// @dev Regression: without this, msg.sender == from skipped allowance consumption entirely.
function test_transferFrom_revert_selfCaller_insufficientAllowance(address from, address to, uint256 amount)
public
{
_assumeValidActor(from);
_assumeValidActor(to);
amount = bound(amount, 1, type(uint256).max);
// No self-approval; allowance[from][from] is 0.

vm.prank(from);
vm.expectRevert(abi.encodeWithSelector(IB20.InsufficientAllowance.selector, from, 0, amount));
token.transferFrom(from, to, amount);
}

/// @notice Verifies transferFrom decreases self-allowance by the spent amount when caller == from
/// @dev Regression: matches OZ / Rust — allowance is spent against `allowances[from][from]`.
function test_transferFrom_success_selfCaller_decreasesAllowance(
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, 1, allowanceAmount);

_mint(from, spendAmount);
vm.prank(from);
token.approve(from, allowanceAmount);

vm.prank(from);
token.transferFrom(from, to, spendAmount);

assertEq(
token.allowance(from, from), allowanceAmount - spendAmount, "self-allowance must decrease by spent amount"
);
assertEq(
uint256(vm.load(address(token), MockB20Storage.allowanceSlot(from, from))),
allowanceAmount - spendAmount,
"allowances[from][from] slot must reflect the consumed amount"
);
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 {
_assumeValidActor(from);
_assumeValidActor(to);
vm.assume(from != to);
amount = bound(amount, 1, type(uint128).max);

_mint(from, amount);
vm.prank(from);
token.approve(from, amount);
_setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID);

vm.prank(from);
token.transferFrom(from, to, amount);

assertEq(token.balanceOf(to), amount, "transfer must succeed despite blocked executor policy");
}
}
54 changes: 54 additions & 0 deletions test/unit/B20/memo/transferFromWithMemo.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,58 @@ contract B20TransferFromWithMemoTest is B20Test {
vm.prank(caller);
assertTrue(token.transferFromWithMemo(from, to, amount, memo), "transferFromWithMemo must return true");
}

// ============================================================
// REGRESSION: SELF-CALLER ALLOWANCE
// ============================================================
//
// Mirrors the transferFrom regression: msg.sender == from MUST still
// consume allowance. See transferFrom.t.sol for the canonical
// regression discussion.

/// @notice Verifies transferFromWithMemo reverts InsufficientAllowance when caller == from and no self-approval exists
/// @dev Regression: inherits the corrected allowance-consumption invariant from transferFrom.
function test_transferFromWithMemo_revert_selfCaller_insufficientAllowance(
address from,
address to,
uint256 amount,
bytes32 memo
) public {
_assumeValidActor(from);
_assumeValidActor(to);
amount = bound(amount, 1, type(uint256).max);

vm.prank(from);
vm.expectRevert(abi.encodeWithSelector(IB20.InsufficientAllowance.selector, from, 0, amount));
token.transferFromWithMemo(from, to, amount, memo);
}

/// @notice Verifies transferFromWithMemo decreases self-allowance by the spent amount when caller == from
/// @dev Regression: spend-tracking parity with transferFrom on the self-caller path.
function test_transferFromWithMemo_success_selfCaller_decreasesAllowance(
address from,
address to,
uint256 amount,
bytes32 memo
) public {
_assumeValidActor(from);
_assumeValidActor(to);
vm.assume(from != to);
amount = bound(amount, 1, type(uint128).max);

_mint(from, amount);
vm.prank(from);
token.approve(from, amount);

vm.prank(from);
token.transferFromWithMemo(from, to, amount, memo);

assertEq(token.allowance(from, from), 0, "self-allowance must be consumed");
assertEq(
uint256(vm.load(address(token), MockB20Storage.allowanceSlot(from, from))),
0,
"allowances[from][from] slot must reflect the consumption"
);
assertEq(token.balanceOf(to), amount, "to must receive the transferred amount");
}
}
Loading