Skip to content

fix(b20): consume allowance on privileged transferFrom (L-04) #133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ilikesymmetry merged 1 commit into from
Jun 2, 2026
Merged

fix(b20): consume allowance on privileged transferFrom (L-04) #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 30 additions & 25 deletions test/lib/mocks/MockB20.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Copy link
Copy Markdown
Collaborator Author

@ilikesymmetry ilikesymmetry Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't add references to tickets, PRs, or finding ids

/// 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
Expand Down
87 changes: 87 additions & 0 deletions test/unit/B20/erc20/transferFrom.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}
}
81 changes: 81 additions & 0 deletions test/unit/B20/memo/transferFromWithMemo.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}
}
Loading